Patchwork D6147: discovery: move cl.hasnode outside of the for-loop

login
register
mail settings
Submitter phabricator
Date March 17, 2019, 3:49 p.m.
Message ID <differential-rev-PHID-DREV-yjv3rn2qcn52jnhwxhw3-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39313/
State Superseded
Headers show

Comments

phabricator - March 17, 2019, 3:49 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  IIUC, resolving attributes for changelog can lead to some overhead. So this
  patch moves that to outside of a for-loop.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6147

AFFECTED FILES
  mercurial/discovery.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - March 18, 2019, 9:28 a.m.
lothiraldan added a comment.


  The series looks good to me after a very quick review. I think a more in-deep review will be necessary.
  
  @pulkit Do you have measured some performance improvement with this series?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6147

To: pulkit, #hg-reviewers
Cc: lothiraldan, mercurial-devel
phabricator - March 18, 2019, 11:30 a.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D6147#89602, @lothiraldan wrote:
  
  > The series looks good to me after a very quick review. I think a more in-deep review will be necessary.
  >
  > @pulkit Do you have measured some performance improvement with this series?
  
  
  I didn't. I am not sure whether this will have any significant performance benefit. I will try to have some numbers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6147

To: pulkit, #hg-reviewers
Cc: lothiraldan, mercurial-devel
phabricator - March 19, 2019, 11:21 a.m.
pulkit added a comment.


  @lothiraldan There is no significant performance benefit which I am able to notice. There is minor improvement of around 0.2 sec on our large repo but that is not stable.
  
  I was reviewing one of the patches when I saw this code and found it needs some cleanup and did that.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6147

To: pulkit, #hg-reviewers
Cc: lothiraldan, mercurial-devel

Patch

diff --git a/mercurial/discovery.py b/mercurial/discovery.py
--- a/mercurial/discovery.py
+++ b/mercurial/discovery.py
@@ -212,14 +212,14 @@ 
     with remote.commandexecutor() as e:
         remotemap = e.callcommand('branchmap', {}).result()
 
+    knownnode = cl.hasnode # do not use nodemap until it is filtered
     # A. register remote heads of branches which are in outgoing set
     for branch, heads in remotemap.iteritems():
         # don't add head info about branches which we don't have locally
         if branch not in branches:
             continue
         known = []
         unsynced = []
-        knownnode = cl.hasnode # do not use nodemap until it is filtered
         for h in heads:
             if knownnode(h):
                 known.append(h)