This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6312

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)
marmoute added a subscriber: marmoute.Jan 23 2020, 12:21 PM

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

baymax requested changes to this revision.Jul 31 2020, 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.Jul 31 2020, 1:56 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/branchmap.py (1 line)

Diff 14927

mercurial/branchmap.py

self[branch] = [cl.node(rev) for rev in bheadrevs] self[branch] = [cl.node(rev) for rev in bheadrevs]
tiprev = bheadrevs[-1] tiprev = bheadrevs[-1]
if tiprev > ntiprev: if tiprev > ntiprev:
ntiprev = tiprev ntiprev = tiprev
if ntiprev > self.tiprev: if ntiprev > self.tiprev:
self.tiprev = ntiprev self.tiprev = ntiprev
self.tipnode = cl.node(ntiprev) self.tipnode = cl.node(ntiprev)
self.filteredhash = scmutil.filteredhash(repo, self.tiprev)
if not self.validfor(repo): if not self.validfor(repo):
# cache key are not valid anymore # cache key are not valid anymore
self.tipnode = nullid self.tipnode = nullid
self.tiprev = nullrev self.tiprev = nullrev
for heads in self.iterheads(): for heads in self.iterheads():
tiprev = max(cl.rev(node) for node in heads) tiprev = max(cl.rev(node) for node in heads)
if tiprev > self.tiprev: if tiprev > self.tiprev: