Page MenuHomePhabricator

branchmap: add a cache validation cache, avoid expensive re-hash on every use
ClosedPublic

Authored by spectral on Tue, Sep 15, 6:48 PM.

Details

Summary

In a pathological hg log case, we end up executing the branchmap validity
checking twice per commit displayed. Or maybe we always do, and I just noticed
because it's really slow in this repo for some reason.

Before:

Time (mean ± σ):      9.816 s ±  0.071 s    [User: 9.435 s, System: 0.392 s]

Range (min … max):    9.709 s …  9.920 s

After:

Time (mean ± σ):      8.671 s ±  0.078 s    [User: 8.309 s, System: 0.392 s]

Range (min … max):    8.594 s …  8.816 s

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

spectral created this revision.Tue, Sep 15, 6:48 PM
pulkit added a subscriber: pulkit.Wed, Sep 16, 3:29 AM

Looks good to me apart from the probable unrelated change.

hg
1 ↗(On Diff #22646)

Unrelated change?

Did you measure the performance impact on other operation than your target operation ? I am curious about the impact of the @property here.

spectral updated this revision to Diff 22658.Wed, Sep 16, 3:14 PM

Did you measure the performance impact on other operation than your target operation ? I am curious about the impact of the @property here.

I did not. Do you have recommendations for a representative sample of repos/commands that might show an issue? Or perhaps I can switch to a different mechanism? There's very few locations that set this, switching them to instead call a 'set_filteredrevs' method would likely be trivial.

Looks good to me apart from the probable unrelated change.

*sigh* I keep doing that... sorry! Updated.

martinvonz accepted this revision.Mon, Sep 21, 4:22 PM
martinvonz added a subscriber: martinvonz.

Did you measure the performance impact on other operation than your target operation ? I am curious about the impact of the @property here.

I did not. Do you have recommendations for a representative sample of repos/commands that might show an issue? Or perhaps I can switch to a different mechanism? There's very few locations that set this, switching them to instead call a 'set_filteredrevs' method would likely be trivial.

I can't think of any myself and this is a clear win in my hg repo, so I'll queue this as is. We can always fix any slowness caused by @property in a follow-up if we can measure any.

This revision is now accepted and ready to land.Mon, Sep 21, 4:22 PM