Patchwork [stable] context: call normal on the right object

login
register
mail settings
Submitter Siddharth Agarwal
Date Aug. 2, 2014, 1:30 a.m.
Message ID <cef74d5ff8d5d2770a1e.1406943040@dev1738.prn1.facebook.com>
Download mbox | patch
Permalink /patch/5220/
State Accepted
Commit 48e32c2c499b6b06c2e1852f3c8d0a4b43a85265
Headers show

Comments

Siddharth Agarwal - Aug. 2, 2014, 1:30 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1406943018 25200
#      Fri Aug 01 18:30:18 2014 -0700
# Branch stable
# Node ID cef74d5ff8d5d2770a1e27c9038fad203040bf19
# Parent  8cab8a5006a5b17727538dd98be8a8704c5fd856
context: call normal on the right object

dirstate.normal is the method that marks files as unchanged/normal.

Rev 20a30cd41d21 started caching dirstate.normal in order to improve
performance. However, there was an error in the patch: taking the wlock, under
some conditions depending on platform, can cause a new dirstate object to be
created. Caching dirstate.normal before calling wlock would then cause the
fixup calls below to be on the old dirstate object, effectively disappearing
into the either.

On Unix and Unix-like OSes, the condition under which we create a new dirstate
object is 'the dirstate file has been modified since the last time we opened
it'. This happens pretty rarely, so the object is usually the same -- there's
little impact.

On Windows, the condition is 'always'. This means files in the lookup state are
never marked normal, so the bug has a serious performance impact since all the
files in the lookup state are re-read every time hg status is run.
Siddharth Agarwal - Aug. 2, 2014, 1:39 a.m.
On 08/01/2014 06:30 PM, Siddharth Agarwal wrote:
> into the either.

Er, 'ether'.
Matt Mackall - Aug. 3, 2014, 2:04 a.m.
On Fri, 2014-08-01 at 18:30 -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1406943018 25200
> #      Fri Aug 01 18:30:18 2014 -0700
> # Branch stable
> # Node ID cef74d5ff8d5d2770a1e27c9038fad203040bf19
> # Parent  8cab8a5006a5b17727538dd98be8a8704c5fd856
> context: call normal on the right object

Queued for stable, thanks.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1341,8 +1341,10 @@ 
             try:
                 # updating the dirstate is optional
                 # so we don't wait on the lock
+                # wlock can invalidate the dirstate, so cache normal _after_
+                # taking the lock
+                wlock = self._repo.wlock(False)
                 normal = self._repo.dirstate.normal
-                wlock = self._repo.wlock(False)
                 try:
                     for f in fixup:
                         normal(f)