Page MenuHomePhabricator

branchcache: update the filteredhash if we update the tiprev
Needs RevisionPublic

Authored by pulkit on Apr 26 2019, 7:22 PM.

Details

Reviewers
baymax
Group Reviewers
hg-reviewers
Summary

We update the tiprev and in next if statement we check whether the branchcache
is valid or not. If the update of tiprev happens, then definitely
self.validfor() will return False because filteredhash is old now.

Let's update the filteredhash if we update the tiprev also. This prevents us
from entering the loop where we iter all the heads, and find the tiprev.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Apr 26 2019, 7:22 PM
yuja added a subscriber: yuja.Apr 28 2019, 4:38 AM
if ntiprev > self.tiprev:
    self.tiprev = ntiprev
    self.tipnode = cl.node(ntiprev)

+ self.filteredhash = scmutil.filteredhash(repo, self.tiprev)

if not self.validfor(repo):
    # cache key are not valid anymore

and update self.filteredhash later again, which smells. Instead, shouldn't
we check the validity first, and take the fast path only if it was valid?

if self.validfor(repo):  # was valid, can take fast path
    self.tiprev = ntiprev
    self.tipnode = cl.node(ntiprev)
else:  # bad luck, recompute tiprev/tipnode
    ...
self.filteredhash = scmutil.filteredhash(repo, self.tiprev)

This seems useful but lingering. @pulkit what's the status of this.

baymax requested changes to this revision.Fri, Jul 31, 1:56 PM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Fri, Jul 31, 1:56 PM