This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1346

fsmonitor: only access inner dirstate map if it is available
ClosedPublic

Authored by mbthomas on Nov 8 2017, 12:31 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGae7ad53d3c8d: fsmonitor: only access inner dirstate map if it is available
rHG5d53247190d3: fsmonitor: only access inner dirstate map if it is available
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mbthomas created this revision.Nov 8 2017, 12:31 PM
durin42 requested changes to this revision.Nov 13 2017, 5:50 PM
durin42 added a subscriber: durin42.
durin42 added inline comments.
hgext/fsmonitor/__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.)

This revision now requires changes to proceed.Nov 13 2017, 5:50 PM
mbthomas added inline comments.Nov 15 2017, 3:44 AM
hgext/fsmonitor/__init__.py
279

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.

mbthomas updated this revision to Diff 3520.Nov 15 2017, 4:08 AM
durin42 accepted this revision.Nov 17 2017, 5:22 PM
This revision is now accepted and ready to land.Nov 17 2017, 5:22 PM
Closed by commit rHG5d53247190d3: fsmonitor: only access inner dirstate map if it is available (authored by mbthomas). · Explain WhyNov 17 2017, 5:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/fsmonitor/__init__.py (6 lines)

Diff 3618

hgext/fsmonitor/__init__.py

clock = 'c:0:0' clock = 'c:0:0'
else: else:
# always ignore # always ignore
ignore = util.always ignore = util.always
dirignore = util.always dirignore = util.always
matchfn = match.matchfn matchfn = match.matchfn
matchalways = match.always() matchalways = match.always()
dmap = self._map._map dmap = self._map
if util.safehasattr(dmap, '_map'):
# for better performance, directly access the inner dirstate map if the
# standard dirstate implementation is in use.
durin42Unsubmitted
Not Done

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.)

durin42: Do we do this for perf reasons? If so, add a comment. (Maybe check and see if this shows up…
mbthomasAuthorUnsubmitted
Not Done

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.

mbthomas: Not sure if I understand the `perfwalk` output. Without this direct access of `dirstate._map.
dmap = dmap._map
nonnormalset = self._map.nonnormalset nonnormalset = self._map.nonnormalset
copymap = self._map.copymap copymap = self._map.copymap
getkind = stat.S_IFMT getkind = stat.S_IFMT
dirkind = stat.S_IFDIR dirkind = stat.S_IFDIR
regkind = stat.S_IFREG regkind = stat.S_IFREG
lnkkind = stat.S_IFLNK lnkkind = stat.S_IFLNK
join = self._join join = self._join