Patchwork manifest: check 'if x is None' instead of 'if not x'

login
register
mail settings
Submitter Durham Goode
Date Feb. 26, 2017, 6:17 p.m.
Message ID <a21671dd2578fea1cb58.1488133051@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/18799/
State Accepted
Headers show

Comments

Durham Goode - Feb. 26, 2017, 6:17 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1488133007 28800
#      Sun Feb 26 10:16:47 2017 -0800
# Node ID a21671dd2578fea1cb58011c392f6dec16f29dc7
# Parent  88203f26ea57627cabd7cf9c4f7843661d6c43ae
manifest: check 'if x is None' instead of 'if not x'

The old code here would end up executing __len__ on a tree manifest to determine
if 'not _data' was true or not. This was very expensive on large repos. Since
this function just cares about memoization, we can just check 'if _data is None'
instead and save a bunch of time.
Yuya Nishihara - Feb. 27, 2017, 12:43 p.m.
On Sun, 26 Feb 2017 10:17:31 -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1488133007 28800
> #      Sun Feb 26 10:16:47 2017 -0800
> # Node ID a21671dd2578fea1cb58011c392f6dec16f29dc7
> # Parent  88203f26ea57627cabd7cf9c4f7843661d6c43ae
> manifest: check 'if x is None' instead of 'if not x'
> 
> The old code here would end up executing __len__ on a tree manifest to determine
> if 'not _data' was true or not. This was very expensive on large repos. Since
> this function just cares about memoization, we can just check 'if _data is None'
> instead and save a bunch of time.

Good catch. Queued, thanks.

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1386,7 +1386,7 @@  class manifestctx(object):
         return self._revlog().parents(self._node)
 
     def read(self):
-        if not self._data:
+        if self._data is None:
             if self._node == revlog.nullid:
                 self._data = manifestdict()
             else:
@@ -1484,7 +1484,7 @@  class treemanifestctx(object):
         return self._repo.manifestlog._revlog.dirlog(self._dir)
 
     def read(self):
-        if not self._data:
+        if self._data is None:
             rl = self._revlog()
             if self._node == revlog.nullid:
                 self._data = treemanifest()