<div dir="ltr"><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 7, 2019 at 4:23 AM Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org">pierre-yves.david@ens-lyon.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 3/6/19 9:35 PM, Yuya Nishihara wrote:<br>
> On Tue, 05 Mar 2019 18:39:15 +0100, Pierre-Yves David wrote:<br>
>> # HG changeset patch<br>
>> # User Pierre-Yves David <<a href="mailto:pierre-yves.david@octobus.net" target="_blank">pierre-yves.david@octobus.net</a>><br>
>> # Date 1551311787 -3600<br>
>> #      Thu Feb 28 00:56:27 2019 +0100<br>
>> # Node ID ffd98d208aa7f92e13bf233b6d752cd2d292ebbe<br>
>> # Parent  82035c1d714f8f3911632ea1271002745fc620f4<br>
>> # EXP-Topic discovery-speedup<br>
>> # Available At <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a><br>
>> #              hg pull <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a> -r ffd98d208aa7<br>
>> discovery: use a lower level but faster way to retrieve parents<br>
> <br>
>> diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py<br>
>> --- a/mercurial/setdiscovery.py<br>
>> +++ b/mercurial/setdiscovery.py<br>
>> @@ -165,6 +165,12 @@ class partialdiscovery(object):<br>
>>           # common.bases and all its ancestors<br>
>>           return self._common.basesheads()<br>
>>   <br>
>> +    def _parentsgetter(self):<br>
>> +        getrev = self._repo.changelog.index.__getitem__<br>
>> +        def getparents(r):<br>
>> +            return getrev(r)[5:6]<br>
>> +        return getparents<br>
> <br>
> Queued, but it's probably better to use revlog._uncheckedparentrevs() than<br>
> peeking a raw index entry. Can you send a follow up?<br>
<br>
Unfortunately, the overhead of _uncheckedparentrevs is still quite <br>
significant. (Attribute access and exception handling)<br>
<br>
before:               66.831393 seconds (median of 3)<br>
my-change:            49.357107 seconds (median of 3)<br>
_uncheckedparentrevs: 58.941629 seconds (median of 3)<br></blockquote><div><br></div><div>These timings are bonkers and I am quite surprised that attribute access and exception handling is adding that much overhead! Are we sure there isn't a `repo.changelog` in a tight loop? Or maybe we're being grossly inefficient about DAG traversals and missing some caching opportunities?<br></div><div><br></div><div>I agree with Yuya that accessing the raw index entries is undesirable. This extremely low-level interface will almost certainly not be supported when we establish a formal storage interface for the changelog. If there are performance concerns here, we *really* need an inline comment to that effect. And it would be great if it pointed to a perf* command that could help measure.</div><div><br></div><div>I'll accept this. But without an inline comment and a documented way to reproduce the performance problem, there's a very good chance we regress this optimization when establishing a formal interface for changelog. Please follow up to help mitigate this possibility.<br></div><div><br></div><div> </div></div></div>