Page MenuHomePhabricator

fsmonitor: only access inner dirstate map if it is available

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



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

rHG Mercurial
Automatic diff as part of commit; lint not applicable.
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.

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

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
This revision was automatically updated to reflect the committed changes.