Patchwork D1346: fsmonitor: only access inner dirstate map if it is available

login
register
mail settings
Submitter phabricator
Date Nov. 8, 2017, 5:31 p.m.
Message ID <differential-rev-PHID-DREV-nce46h7dofgtns2hsdvz-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25429/
State Superseded
Headers show

Comments

phabricator - Nov. 8, 2017, 5:31 p.m.
mbthomas created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  As part of the dirstate refactor, fsmonitor was updated to directly access the
  inner map of the dirstatemap object.
  
  Dirstatemap reimplementations may not use a map like this, so only access it if
  it is there.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1346

AFFECTED FILES
  hgext/fsmonitor/__init__.py

CHANGE DETAILS




To: mbthomas, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 13, 2017, 10:50 p.m.
durin42 requested changes to this revision.
durin42 added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> __init__.py:279
> +        # directly access the inner dirstate map if the standard dirstate
> +        # implementation is in use.
> +        dmap = dmap._map

Do we do this for perf reasons?  If so, add a comment.

(Maybe check and see if this shows up on perfdirstate or similar - if it does, document that in the commit message with numbers, and then you don't even need a comment because we'll find that with blame.)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1346

To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
phabricator - Nov. 15, 2017, 8:44 a.m.
mbthomas added inline comments.

INLINE COMMENTS

> durin42 wrote in __init__.py:279
> Do we do this for perf reasons?  If so, add a comment.
> 
> (Maybe check and see if this shows up on perfdirstate or similar - if it does, document that in the commit message with numbers, and then you don't even need a comment because we'll find that with blame.)

Not sure if I understand the `perfwalk` output.  Without this direct access of `dirstate._map._map` I get:

  ! result: 1485
  ! wall 0.004521 comb 0.000000 user 0.000000 sys 0.000000 (best of 589)

with the direct access:

  ! result: 1485
  ! wall 0.004203 comb 0.000000 user 0.000000 sys 0.000000 (best of 628)

I'm guessing the wall clock time is important, so this is a ~7% improvement.  Is that right?

I will add a comment here in any case.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1346

To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mercurial-devel

Patch

diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py
--- a/hgext/fsmonitor/__init__.py
+++ b/hgext/fsmonitor/__init__.py
@@ -273,7 +273,11 @@ 
 
     matchfn = match.matchfn
     matchalways = match.always()
-    dmap = self._map._map
+    dmap = self._map
+    if util.safehasattr(dmap, '_map'):
+        # directly access the inner dirstate map if the standard dirstate
+        # implementation is in use.
+        dmap = dmap._map
     nonnormalset = self._map.nonnormalset
 
     copymap = self._map.copymap