Patchwork branchmap: check node against nodemap instead of changelog (for perf)

login
register
mail settings
Submitter Durham Goode
Date March 3, 2016, 11:52 p.m.
Message ID <37770ef41a955e687b48.1457049179@dev8486.prn1.facebook.com>
Download mbox | patch
Permalink /patch/13586/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Durham Goode - March 3, 2016, 11:52 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1457032193 28800
#      Thu Mar 03 11:09:53 2016 -0800
# Node ID 37770ef41a955e687b481e955c251e7cd0386c27
# Parent  549ff28a345f595cad7e06fb08c2ac6973e2f030
branchmap: check node against nodemap instead of changelog (for perf)

Testing 'node in repo' requires constructing a changectx, which is a little
expensive.  Testing 'node in repo.changelog.nodemap' is notably faster. This
saves 10-20ms off of every command, when testing a few thousand nodes from the
branch cache.

I considered changing the implementation of localrepository.__contains__ so
every place would benefit from the change, but since
localrepository.__contains__ uses changectx to check if the commit exists, it
means it supports a wider range of possible inputs (like revs, hashes, '.',
etc), so it seemed unnecessarily risky.
Yuya Nishihara - March 5, 2016, 9:40 a.m.
On Thu, 3 Mar 2016 15:52:59 -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1457032193 28800
> #      Thu Mar 03 11:09:53 2016 -0800
> # Node ID 37770ef41a955e687b481e955c251e7cd0386c27
> # Parent  549ff28a345f595cad7e06fb08c2ac6973e2f030
> branchmap: check node against nodemap instead of changelog (for perf)
> 
> Testing 'node in repo' requires constructing a changectx, which is a little
> expensive.  Testing 'node in repo.changelog.nodemap' is notably faster. This
> saves 10-20ms off of every command, when testing a few thousand nodes from the
> branch cache.

changelog.nodemap bypasses the filtering. If the construction of changectx is
costly, changelog.hasnode() could be used.

Patch

diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -55,6 +55,7 @@  def read(repo):
         if not partial.validfor(repo):
             # invalidate the cache
             raise ValueError('tip differs')
+        nodemap = repo.changelog.nodemap
         for l in lines:
             if not l:
                 continue
@@ -62,9 +63,9 @@  def read(repo):
             if state not in 'oc':
                 raise ValueError('invalid branch state')
             label = encoding.tolocal(label.strip())
-            if not node in repo:
-                raise ValueError('node %s does not exist' % node)
             node = bin(node)
+            if not node in nodemap:
+                raise ValueError('node %s does not exist' % hex(node))
             partial.setdefault(label, []).append(node)
             if state == 'c':
                 partial._closednodes.add(node)