<div dir="ltr"><br><br><div class="gmail_quote">On Sat, Mar 7, 2015 at 9:25 AM Augie Fackler <<a href="mailto:raf@durin42.com">raf@durin42.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"># HG changeset patch<br>
# User Augie Fackler <<a href="mailto:augie@google.com" target="_blank">augie@google.com</a>><br>
# Date 1425747879 18000<br>
# Sat Mar 07 12:04:39 2015 -0500<br>
# Node ID 74e64852b07f9cfb5a7b89d827dd9e<u></u>1f01314b1b<br>
# Parent 2524c117ce836e18402cac792361f0<u></u>e44cac42c5<br>
manifest: split manifestdict into high-level and low-level logic<br>
<br>
The low-level logic type (_lazymanifest) matches the behavior of the C<br>
implementation introduced in a5f1bccd. A future patch will use that<br>
when available.<br>
<br>
diff --git a/mercurial/manifest.py b/mercurial/manifest.py<br>
--- a/mercurial/manifest.py<br>
+++ b/mercurial/manifest.py<br>
@@ -9,53 +9,119 @@ from i18n import _<br>
import mdiff, parsers, error, revlog, util<br>
import array, struct<br>
<br>
-# Pure Python fallback<br>
-def _parsemanifest(mfdict, fdict, lines):<br>
- bin = revlog.bin<br>
- for l in lines.splitlines():<br>
- f, n = l.split('\0')<br>
- if len(n) > 40:<br>
- fdict[f] = n[40:]<br>
- mfdict[f] = bin(n[:40])<br>
- else:<br>
- mfdict[f] = bin(n)<br>
<br>
-def _parse(lines, mfdict, flags):<br>
- try:<br>
- parsers.parse_manifest(mfdict, flags, lines)<br>
- except AttributeError:<br>
- _parsemanifest(mfdict, flags, lines)<br>
- return mfdict<br>
+class _lazymanifest(dict):<br>
+ """This is the pure implementation of lazymanifest.<br>
<br>
-class manifestdict(dict):<br>
- def __init__(self, data=''):<br>
- self._flags = {}<br>
- _parse(data, self, self._flags)<br>
+ It has not been optimized *at all* and is not lazy.<br>
+ """<br>
+<br>
+ def __init__(self, data):<br>
+ # This init method does a little bit of excessive-looking<br>
+ # precondition checking. This is so that the behavior of this<br>
+ # class exactly matches its C counterpart to try and help<br>
+ # prevent surprise breakage for anyone that develops against<br>
+ # the pure version.<br>
+ if data and data[-1] != '\n':<br>
+ raise ValueError('Manifest did not end in a newline.')<br>
+ dict.__init__(self)<br>
+ prev = None<br>
+ for l in data.splitlines():<br>
+ if prev is not None and prev > l:<br>
+ raise ValueError('Manifest lines not in sorted order.')<br>
+ prev = l<br>
+ f, n = l.split('\0')<br>
+ if len(n) > 40:<br>
+ self[f] = revlog.bin(n[:40]), n[40:]<br>
+ else:<br>
+ self[f] = revlog.bin(n), ''<br>
<br>
def __setitem__(self, k, v):<br>
- assert v is not None<br>
- dict.__setitem__(self, k, v)<br>
- def flags(self, f):<br>
- return self._flags.get(f, "")<br>
- def setflag(self, f, flags):<br>
- """Set the flags (symlink, executable) for path f."""<br>
- self._flags[f] = flags<br>
+ node, flag = v<br>
+ assert node is not None<br>
+ if len(node) > 21:<br>
+ node = node[:21] # match c implementation behavior<br>
+ dict.__setitem__(self, k, (node, flag))<br>
+<br>
+ def __iter__(self):<br>
+ return ((f, e[0], e[1]) for f, e in sorted(self.iteritems()))<br></blockquote><div><br></div><div>What's the reason for yielding (f, n, fl) instead of the more natural (f, (n, fl))? The infamous effect tuples have on Python's GC? Also see questions further down.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
def copy(self):<br>
- copy = manifestdict()<br>
- dict.__init__(copy, self)<br>
- copy._flags = dict.copy(self._flags)<br>
- return copy<br>
+ c = _lazymanifest('')<br>
+ c.update(self)<br>
+ return c<br>
+<br>
+ def diff(self, m2, clean=False):<br>
+ '''Finds changes between the current manifest and m2.'''<br>
+ diff = {}<br>
+<br>
+ for fn, e1 in self.iteritems():<br>
+ if fn not in m2:<br>
+ diff[fn] = e1, (None, '')<br>
+ else:<br>
+ e2 = m2[fn]<br>
+ if e1 != e2:<br>
+ diff[fn] = e1, e2<br>
+ elif clean:<br>
+ diff[fn] = None<br>
+<br>
+ for fn, e2 in m2.iteritems():<br>
+ if fn not in self:<br>
+ diff[fn] = (None, ''), e2<br>
+<br>
+ return diff<br>
+<br>
+ def filtercopy(self, filterfn):<br>
+ c = _lazymanifest('')<br>
+ for f, n, fl in self:<br>
+ if filterfn(f):<br>
+ c[f] = n, fl<br>
+ return c<br>
+<br>
+ def text(self):<br>
+ """Get the full data of this manifest as a bytestring."""<br>
+ fl = sorted(self)<br>
+<br>
+ _hex = revlog.hex<br>
+ # if this is changed to support newlines in filenames,<br>
+ # be sure to check the templates/ dir again (especially *-raw.tmpl)<br>
+ return ''.join("%s\0%s%s\n" % (<br>
+ f, _hex(n[:20]), flag) for f, n, flag in fl)<br>
+<br>
+class manifestdict(object):<br></blockquote><div><br></div><div>So <span style="font-size:13.1999998092651px">_lazymanifest is not lazy, manifestdict is not a dict, but </span><span style="font-size:13.1999998092651px">_lazymanifest is a dict :-) Just a note, not asking for anything to change.</span></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ def __init__(self, data=''):<br>
+ self._lm = _lazymanifest(data)<br>
+<br>
+ def __getitem__(self, key):<br>
+ return self._lm[key][0]<br>
+<br>
+ def __len__(self):<br>
+ return len(self._lm)<br>
+<br>
+ def __setitem__(self, key, node):<br>
+ self._lm[key] = node, self.flags(key, '')<br>
+<br>
+ def __contains__(self, key):<br>
+ return key in self._lm<br>
+<br>
+ def __delitem__(self, key):<br>
+ del self._lm[key]<br>
+<br>
+ def keys(self):<br>
+ return [x[0] for x in self._lm]<br>
+<br>
+ def iterkeys(self):<br>
+ return iter(self.keys())<br>
+<br>
def intersectfiles(self, files):<br>
- '''make a new manifestdict with the intersection of self with files<br>
+ '''make a new lazymanifest with the intersection of self with files<br>
<br>
The algorithm assumes that files is much smaller than self.'''<br>
ret = manifestdict()<br>
+ lm = self._lm<br>
for fn in files:<br>
- if fn in self:<br>
- ret[fn] = self[fn]<br>
- flags = self._flags.get(fn, None)<br>
- if flags:<br>
- ret._flags[fn] = flags<br>
+ if fn in lm:<br>
+ ret._lm[fn] = self._lm[fn]<br>
return ret<br>
<br>
def filesnotin(self, m2):<br>
@@ -74,11 +140,9 @@ class manifestdict(dict):<br>
(not match.anypats() and util.all(fn in self for fn in files))):<br>
return self.intersectfiles(files)<br>
<br>
- m = self.copy()<br>
- for fn in m.keys():<br>
- if not match(fn):<br>
- del m[fn]<br>
- return m<br>
+ lm = manifestdict('')<br>
+ lm._lm = self._lm.filtercopy(match)<br>
+ return lm<br>
<br>
def diff(self, m2, clean=False):<br>
'''Finds changes between the current manifest and m2.<br>
@@ -95,35 +159,36 @@ class manifestdict(dict):<br>
the nodeid will be None and the flags will be the empty<br>
string.<br>
'''<br>
- diff = {}<br>
+ return self._lm.diff(m2._lm, clean)<br>
<br>
- for fn, n1 in self.iteritems():<br>
- fl1 = self._flags.get(fn, '')<br>
- n2 = m2.get(fn, None)<br>
- fl2 = m2._flags.get(fn, '')<br>
- if n2 is None:<br>
- fl2 = ''<br>
- if n1 != n2 or fl1 != fl2:<br>
- diff[fn] = ((n1, fl1), (n2, fl2))<br>
- elif clean:<br>
- diff[fn] = None<br>
+ def setflag(self, key, flag):<br>
+ self._lm[key] = self[key], flag<br>
<br>
- for fn, n2 in m2.iteritems():<br>
- if fn not in self:<br>
- fl2 = m2._flags.get(fn, '')<br>
- diff[fn] = ((None, ''), (n2, fl2))<br>
+ def get(self, key, default=None):<br>
+ try:<br>
+ return self._lm[key][0]<br>
+ except KeyError:<br>
+ return default<br>
<br>
- return diff<br>
+ def flags(self, key, default=''):<br>
+ try:<br>
+ return self._lm[key][1]<br>
+ except KeyError:<br>
+ return default<br>
+<br>
+ def __iter__(self):<br>
+ return (x[0] for x in self._lm)<br></blockquote><div><br></div><div>If the answer to my question above was about GC, then wouldn't this (the creation of tuples that are immediately thrown away) be an equally big problem? If the low-level type's __iter__ was left untouched <span style="font-size:13.1999998092651px">(and iteritems() would be used where __iter__ is currently used)</span><span style="font-size:13.1999998092651px">, then there wouldn't be any tuples involved here.</span></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ def copy(self):<br>
+ c = manifestdict('')<br>
+ c._lm = self._lm.copy()<br>
+ return c<br>
+<br>
+ def iteritems(self):<br>
+ return (x[:2] for x in self._lm)<br></blockquote><div><br></div><div>Similar comment here: this creates a 3-tuple that gets converted into a 2-tuple. That does seem better than creating 2 2-tuples (f, (n, e)) that converted into another one (f, n). Even better would be to add a new filesandnodes() generator that yields exactly the wanted (f, n) tuples, no? OTOH, is that really what we want in many places? I have a feeling that most places that user iteritems() would actually prefer all three fields. Yielding (f, (n, e)) seems cleaner to me, but if GC is a concern, then perhaps we'd have to yield (f, n, e). What do you think?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
def text(self):<br>
- """Get the full data of this manifest as a bytestring."""<br>
- fl = sorted(self)<br>
- _checkforbidden(fl)<br>
-<br>
- hex, flags = revlog.hex, self.flags<br>
- # if this is changed to support newlines in filenames,<br>
- # be sure to check the templates/ dir again (especially *-raw.tmpl)<br>
- return ''.join("%s\0%s%s\n" % (f, hex(self[f]), flags(f)) for f in fl)<br>
+ return self._lm.text()<br>
<br>
def fastdelta(self, base, changes):<br>
"""Given a base manifest text as an array.array and a list of changes<br>
@@ -143,7 +208,8 @@ class manifestdict(dict):<br>
# bs will either be the index of the item or the insert point<br>
start, end = _msearch(addbuf, f, start)<br>
if not todelete:<br>
- l = "%s\0%s%s\n" % (f, revlog.hex(self[f]), self.flags(f))<br>
+ h, fl = self._lm[f]<br>
+ l = "%s\0%s%s\n" % (f, revlog.hex(h), fl)<br>
else:<br>
if start == end:<br>
# item we want to delete was not found, error out<br>
@@ -280,12 +346,10 @@ class manifest(revlog.revlog):<br>
m = self._mancache[node][0]<br>
return m.get(f), m.flags(f)<br>
text = self.revision(node)<br>
- start, end = _msearch(text, f)<br>
- if start == end:<br>
+ try:<br>
+ return manifestdict(text)._lm[f]<br>
+ except KeyError:<br>
return None, None<br>
- l = text[start:end]<br>
- f, n = l.split('\0')<br>
- return revlog.bin(n[:40]), n[40:-1]<br>
<br>
def add(self, m, transaction, link, p1, p2, added, removed):<br>
if p1 in self._mancache:<br>
diff --git a/tests/test-manifest.py b/tests/test-manifest.py<br>
--- a/tests/test-manifest.py<br>
+++ b/tests/test-manifest.py<br>
@@ -4,7 +4,7 @@ import itertools<br>
<br>
import silenttestrunner<br>
<br>
-from mercurial import parsers<br>
+from mercurial import manifest as manifestmod<br>
<br>
HASH_1 = '1' * 40<br>
HASH_2 = 'f' * 40<br>
@@ -38,12 +38,12 @@ class testmanifest(unittest.<u></u>TestCase):<br>
self.assert_(thing in container, msg)<br>
<br>
def testEmptyManifest(self):<br>
- m = parsers.lazymanifest('')<br>
+ m = manifestmod._lazymanifest('')<br>
self.assertEqual(0, len(m))<br>
self.assertEqual([], list(m))<br>
<br>
def testManifest(self):<br>
- m = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
want = [<br>
('bar/baz/qux.py', binascii.unhexlify(HASH_2), 'l'),<br>
('foo', binascii.unhexlify(HASH_1), ''),<br>
@@ -58,13 +58,13 @@ class testmanifest(unittest.<u></u>TestCase):<br>
def testSetItem(self):<br>
want = binascii.unhexlify(HASH_1), ''<br>
<br>
- m = parsers.lazymanifest('')<br>
+ m = manifestmod._lazymanifest('')<br>
m['a'] = want<br>
self.assertIn('a', m)<br>
self.assertEqual(want, m['a'])<br>
self.assertEqual('a\0' + HASH_1 + '\n', m.text())<br>
<br>
- m = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
m['a'] = want<br>
self.assertEqual(want, m['a'])<br>
self.assertEqual('a\0' + HASH_1 + '\n' + A_SHORT_MANIFEST,<br>
@@ -76,7 +76,7 @@ class testmanifest(unittest.<u></u>TestCase):<br>
def testCompaction(self):<br>
unhex = binascii.unhexlify<br>
h1, h2 = unhex(HASH_1), unhex(HASH_2)<br>
- m = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
m['alpha'] = h1, ''<br>
m['beta'] = h2, ''<br>
del m['foo']<br>
@@ -91,8 +91,8 @@ class testmanifest(unittest.<u></u>TestCase):<br>
self.assertEqual(w, list(m))<br>
<br>
def testSetGetNodeSuffix(self):<br>
- clean = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
- m = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ clean = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
h, f = m['foo']<br>
want = h + 'a', f<br>
# Merge code wants to set 21-byte fake hashes at times<br>
@@ -120,7 +120,7 @@ class testmanifest(unittest.<u></u>TestCase):<br>
self.assertEqual({'foo': ((h, ''), want)}, clean.diff(m))<br>
<br>
def testFilterCopyException(self):<br>
- m = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
def filt(path):<br>
if path == 'foo':<br>
assert False<br>
@@ -128,7 +128,7 @@ class testmanifest(unittest.<u></u>TestCase):<br>
self.assertRaises(<u></u>AssertionError, m.filtercopy, filt)<br>
<br>
def testRemoveItem(self):<br>
- m = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
del m['foo']<br>
self.assertRaises(KeyError, lambda : m['foo'])<br>
self.assertEqual(1, len(m))<br>
@@ -138,9 +138,9 @@ class testmanifest(unittest.<u></u>TestCase):<br>
MISSING = (None, '')<br>
addl = 'z-only-in-left\0' + HASH_1 + '\n'<br>
addr = 'z-only-in-right\0' + HASH_2 + 'x\n'<br>
- left = parsers.lazymanifest(<br>
+ left = manifestmod._lazymanifest(<br>
A_SHORT_MANIFEST.replace(HASH_<u></u>1, HASH_3 + 'x') + addl)<br>
- right = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST + addr)<br>
+ right = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST + addr)<br>
want = {<br>
'foo': ((binascii.unhexlify(HASH_3), 'x'),<br>
(binascii.unhexlify(HASH_1), '')),<br>
@@ -154,14 +154,14 @@ class testmanifest(unittest.<u></u>TestCase):<br>
'foo': (MISSING, (binascii.unhexlify(HASH_3), 'x')),<br>
'z-only-in-left': (MISSING, (binascii.unhexlify(HASH_1), '')),<br>
}<br>
- self.assertEqual(want, parsers.lazymanifest('').diff(<u></u>left))<br>
+ self.assertEqual(want, manifestmod._lazymanifest('').<u></u>diff(left))<br>
<br>
want = {<br>
'bar/baz/qux.py': ((binascii.unhexlify(HASH_2), 'l'), MISSING),<br>
'foo': ((binascii.unhexlify(HASH_3), 'x'), MISSING),<br>
'z-only-in-left': ((binascii.unhexlify(HASH_1), ''), MISSING),<br>
}<br>
- self.assertEqual(want, left.diff(parsers.<u></u>lazymanifest('')))<br>
+ self.assertEqual(want, left.diff(manifestmod._<u></u>lazymanifest('')))<br>
copy = right.copy()<br>
del copy['z-only-in-right']<br>
del right['foo']<br>
@@ -171,7 +171,7 @@ class testmanifest(unittest.<u></u>TestCase):<br>
}<br>
self.assertEqual(want, right.diff(copy))<br>
<br>
- short = parsers.lazymanifest(A_SHORT_<u></u>MANIFEST)<br>
+ short = manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST)<br>
pruned = short.copy()<br>
del pruned['foo']<br>
want = {<br>
@@ -192,30 +192,37 @@ class testmanifest(unittest.<u></u>TestCase):<br>
backwards = ''.join(<br>
l + '\n' for l in reversed(A_SHORT_MANIFEST.<u></u>split('\n')) if l)<br>
try:<br>
- parsers.lazymanifest(<u></u>backwards)<br>
+ manifestmod._lazymanifest(<u></u>backwards)<br>
self.fail('Should have raised ValueError')<br>
except ValueError, v:<br>
self.assertIn('Manifest lines not in sorted order.', str(v))<br>
<br>
def testNoTerminalNewline(self):<br>
try:<br>
- parsers.lazymanifest(A_SHORT_<u></u>MANIFEST + 'wat')<br>
+ manifestmod._lazymanifest(A_<u></u>SHORT_MANIFEST + 'wat')<br>
self.fail('Should have raised ValueError')<br>
except ValueError, v:<br>
self.assertIn('Manifest did not end in a newline.', str(v))<br>
<br>
def testNoNewLineAtAll(self):<br>
try:<br>
- parsers.lazymanifest('wat')<br>
+ manifestmod._lazymanifest('<u></u>wat')<br>
self.fail('Should have raised ValueError')<br>
except ValueError, v:<br>
self.assertIn('Manifest did not end in a newline.', str(v))<br>
<br>
def testHugeManifest(self):<br>
- m = parsers.lazymanifest(A_HUGE_<u></u>MANIFEST)<br>
+ m = manifestmod._lazymanifest(A_<u></u>HUGE_MANIFEST)<br>
self.assertEqual(HUGE_<u></u>MANIFEST_ENTRIES, len(m))<br>
self.assertEqual(len(m), len(list(m)))<br>
<br>
+ def testIntersectFiles(self):<br>
+ m = manifestmod.manifestdict(A_<u></u>HUGE_MANIFEST)<br>
+ m2 = m.intersectfiles(['file1', 'file200', 'file300'])<br>
+ w = ('file1\0%sx\n'<br>
+ 'file200\0%sl\n'<br>
+ 'file300\0%s\n') % (HASH_2, HASH_1, HASH_1)<br>
+ self.assertEqual(w, m2.text())<br>
<br>
if __name__ == '__main__':<br>
silenttestrunner.main(__name__<u></u>)<br>
______________________________<u></u>_________________<br>
Mercurial-devel mailing list<br>
<a href="mailto:Mercurial-devel@selenic.com" target="_blank">Mercurial-devel@selenic.com</a><br>
<a href="http://selenic.com/mailman/listinfo/mercurial-devel" target="_blank">http://selenic.com/mailman/<u></u>listinfo/mercurial-devel</a><br>
</blockquote></div></div>