| Submitter | Durham Goode |
|---|---|
| Date | Feb. 4, 2016, 1:44 a.m. |
| Message ID | <5211c5eee88649263624.1454550281@dev8486.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/12969/ |
| State | Accepted |
| Delegated to: | Yuya Nishihara |
| Headers | show |
Comments
Durham Goode <durham@fb.com> writes: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1454550251 28800 > # Wed Feb 03 17:44:11 2016 -0800 > # Node ID 5211c5eee88649263624a82e65dc53e6f3882ba3 > # Parent 01a5143cd25f285f8c745a92986cd7186bb32c90 > memctx: fix memctx manifest file hashes > > When memctx is asked for a manifest, it constructs one by merging the p1 > manifest, and the changes that are on top. For the changes on top, it was > previously using p1.node() as the file entries parent, which actually returns > the commit node that the p1 linkrev points at! Which is entirely incorrect. > > The fix is to use p1.filenode() instead, which returns the parent file node as > desired. > > I don't know how to execute this or make it have a visible effect, so I'm not > sure how to test it. It was noticed because asking for the linkrev is an > expensive operation when using the remotefilelog extension and this was causing > performance regressions with commit. Huh. This is almost certainly my fault for misunderstanding node vs filenode. I just ran this patch against our internal memctx at Bitbucket and it passed this change seems fine to me.
On Wed, 3 Feb 2016 17:44:41 -0800, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1454550251 28800 > # Wed Feb 03 17:44:11 2016 -0800 > # Node ID 5211c5eee88649263624a82e65dc53e6f3882ba3 > # Parent 01a5143cd25f285f8c745a92986cd7186bb32c90 > memctx: fix memctx manifest file hashes Pushed to the clowncopter as per Sean's review, thanks.
Patch
diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -1892,9 +1892,9 @@ class memctx(committablectx): p2node = nullid p = pctx[f].parents() # if file isn't in pctx, check p2? if len(p) > 0: - p1node = p[0].node() + p1node = p[0].filenode() if len(p) > 1: - p2node = p[1].node() + p2node = p[1].filenode() man[f] = revlog.hash(self[f].data(), p1node, p2node) for f in self._status.added: