This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6711

branchheads: store wdir-dependent caches in wcache (issue6181)
AbandonedPublic

Authored by spectral on Aug 2 2019, 9:26 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

Previously, all branch2-* caches were stored in .hg/cache, which is shared
across repos when using hg share. This can cause cache thrashing when the
repos are accessed concurrently, since some of the caches depend on wdir state
(which is NOT shared across the repos).

There's already a cache directory for caches that depend on wdir state, wcache,
so let's just put the caches there.

This change does not clean up any existing caches in .hg/cache that are now
moved to the new location, and it does not re-use the caches from that location
when constructing them in the wcache directory. Upgrades to versions of
Mercurial that have this commit will need to regenerate their branchheads once, and
this may be an expensive operation.

The format of these cache files is not changing - forwards/backwards
compatibility remains; if a newer repo (with these in wcache) is accessed by an
older hg, the older hg will either see the files that remain in .hg/cache, and
update them there, or will not see the files at all and will fall back to a
slower path (and then likely cache them in .hg/cache). If it's then accessed by
a newer hg, the work done by the older hg is ignored, and the wcache copies are
brought up to date.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Aug 2 2019, 9:26 PM
marmoute added a subscriber: marmoute.Aug 3 2019, 7:49 AM

The change looks good to me, However we probably want to introduce a new filter level 'wdir-independent-visible' to ensure we a good branchcache cache in .hg/cache that most share can use and that will be kept up to date. This also means we need to make sure it is warmed after transaction.

spectral updated this revision to Diff 16130.Aug 5 2019, 9:07 PM
spectral added a comment.Aug 5 2019, 9:11 PM
In D6711#98248, @marmoute wrote:

The change looks good to me, However we probably want to introduce a new filter level 'wdir-independent-visible' to ensure we a good branchcache cache in .hg/cache that most share can use and that will be kept up to date. This also means we need to make sure it is warmed after transaction.

I agree such a thing would be useful, and pretty easy to add (just needs to not remove items considered "pinned" during computehidden, but I'm a little concerned about the proliferation of these things. I think ideally we'd end up with:

visible-hidden > visible > wdir-independent-visible > wdir-independent-served > wdir-independent-served.hidden > immutable > base
and
served > wdir-independent-served > wdir-independent-served.hidden > immutable > base
and
served.hidden > wdir-independent-served.hidden > immutable > base

I think?

(Not sure if we need a wdir-independent-visible-hidden, I still don't really understand visible-hidden, I just know that updating it actually breaks a Lot of stuff, so I avoided doing so in the first commit in the stack. :))

marmoute added a comment.Aug 8 2019, 1:01 PM
In D6711#98318, @spectral wrote:
In D6711#98248, @marmoute wrote:

The change looks good to me, However we probably want to introduce a new filter level 'wdir-independent-visible' to ensure we a good branchcache cache in .hg/cache that most share can use and that will be kept up to date. This also means we need to make sure it is warmed after transaction.

I agree such a thing would be useful, and pretty easy to add (just needs to not remove items considered "pinned" during computehidden, but I'm a little concerned about the proliferation of these things. I think ideally we'd end up with:
visible-hidden > visible > wdir-independent-visible > wdir-independent-served > wdir-independent-served.hidden > immutable > base
and
served > wdir-independent-served > wdir-independent-served.hidden > immutable > base
and
served.hidden > wdir-independent-served.hidden > immutable > base
I think?

Yeah, this makes a lot of them, but the alternative (having non-up-to-date cache in the share-source would a significant issue). In the current situation you fix one bug, reintroducing another. I wonder if we have a "simpler or saner" way to introduce these various disctinction. Let me think about it a bit.

(Not sure if we need a wdir-independent-visible-hidden, I still don't really understand visible-hidden, I just know that updating it actually breaks a Lot of stuff, so I avoided doing so in the first commit in the stack. :))

visible-hidden still exclude all the internal phase commit that we never want to expose to anyone. (yeah the name is … confusing)

spectral planned changes to this revision.Sep 5 2019, 3:06 PM

I'm not sure what the current state of this is, but I don't want it lingering in the review queue while I'm not actively working on it.

spectral abandoned this revision.Apr 19 2021, 3:04 PM

Revision Contents
Changeset List

Diff 16130

mercurial/branchmap.py

""" checks whether a branch of this name exists or not """ """ checks whether a branch of this name exists or not """
self._verifybranch(label) self._verifybranch(label)
return label in self._entries return label in self._entries
@classmethod @classmethod
def fromfile(cls, repo): def fromfile(cls, repo):
f = None f = None
try: try:
f = repo.cachevfs(cls._filename(repo)) f = cls._opencachefile(repo)
lineiter = iter(f) lineiter = iter(f)
cachekey = next(lineiter).rstrip('\n').split(" ", 2) cachekey = next(lineiter).rstrip('\n').split(" ", 2)
last, lrev = cachekey[:2] last, lrev = cachekey[:2]
last, lrev = bin(last), int(lrev) last, lrev = bin(last), int(lrev)
filteredhash = None filteredhash = None
hasnode = repo.changelog.hasnode hasnode = repo.changelog.hasnode
if len(cachekey) > 2: if len(cachekey) > 2:
filteredhash = bin(cachekey[2]) filteredhash = bin(cachekey[2])
raise ValueError(r'invalid branch state') raise ValueError(r'invalid branch state')
label = encoding.tolocal(label.strip()) label = encoding.tolocal(label.strip())
node = bin(node) node = bin(node)
self._entries.setdefault(label, []).append(node) self._entries.setdefault(label, []).append(node)
if state == 'c': if state == 'c':
self._closednodes.add(node) self._closednodes.add(node)
@staticmethod @staticmethod
def _filename(repo): def _opencachefile(repo, *args, **kwargs):
"""name of a branchcache file for a given repo or repoview""" """name of a branchcache file for a given repo or repoview"""
filename = "branch2" filename = "branch2"
vfs = repo.cachevfs
if repo.filtername: if repo.filtername:
filename = '%s-%s' % (filename, repo.filtername) filename = '%s-%s' % (filename, repo.filtername)
return filename if repo.filtername in repoviewutil.wdirdependent:
vfs = repo.wcachevfs
return vfs(filename, *args, **kwargs)
def validfor(self, repo): def validfor(self, repo):
"""Is the cache content valid regarding a repo """Is the cache content valid regarding a repo
- False when cached tipnode is unknown or if we detect a strip. - False when cached tipnode is unknown or if we detect a strip.
- True when cache is up to date or a subset of current repo.""" - True when cache is up to date or a subset of current repo."""
try: try:
return ((self.tipnode == repo.changelog.node(self.tiprev)) return ((self.tipnode == repo.changelog.node(self.tiprev))
def copy(self): def copy(self):
"""return an deep copy of the branchcache object""" """return an deep copy of the branchcache object"""
return type(self)( return type(self)(
self._entries, self.tipnode, self.tiprev, self.filteredhash, self._entries, self.tipnode, self.tiprev, self.filteredhash,
self._closednodes) self._closednodes)
def write(self, repo): def write(self, repo):
try: try:
f = repo.cachevfs(self._filename(repo), "w", atomictemp=True) f = self._opencachefile(repo, "w", atomictemp=True)
cachekey = [hex(self.tipnode), '%d' % self.tiprev] cachekey = [hex(self.tipnode), '%d' % self.tiprev]
if self.filteredhash is not None: if self.filteredhash is not None:
cachekey.append(hex(self.filteredhash)) cachekey.append(hex(self.filteredhash))
f.write(" ".join(cachekey) + '\n') f.write(" ".join(cachekey) + '\n')
nodecount = 0 nodecount = 0
for label, nodes in sorted(self._entries.iteritems()): for label, nodes in sorted(self._entries.iteritems()):
label = encoding.fromlocal(label) label = encoding.fromlocal(label)
for node in nodes: for node in nodes:

mercurial/utils/repoviewutil.py

'served': 'immutable', 'served': 'immutable',
'immutable': 'base'} 'immutable': 'base'}
# List of "heads" from `subsettable`: filters that do not have any superset that # List of "heads" from `subsettable`: filters that do not have any superset that
# would cause them to be populated, so they must be populated explicitly. This # would cause them to be populated, so they must be populated explicitly. This
# can be calculated from `subsettable` but `subsettable` is essentially # can be calculated from `subsettable` but `subsettable` is essentially
# constant, so we just hard-code it. # constant, so we just hard-code it.
subsettableheads = frozenset({'visible-hidden', 'served.hidden'}) subsettableheads = frozenset({'visible-hidden', 'served.hidden'})
# List of views that are potentially dependent upon wdir state (such as the
# current parents of the working directory) and need to be kept in wcache
# instead of cache.
wdirdependent = frozenset(['served', 'visible', 'visible-hidden'])

tests/test-branchmap-cache.t

  • This file was added.
$ cat >> $HGRCPATH << EOF
> [extensions]
> share=
> [experimental]
> evolution=createmarkers
> EOF
$ hg init -q orig
$ cd orig
$ echo hi > foo
$ hg ci -qAm initial_rev0
$ echo "hi again" >> foo
$ hg ci -qAm "hi again rev1"
$ hg commit -q --amend -m "obsoleted 1, this is rev2"
$ cd ..
$ hg share -q orig other
$ cd other
$ hg co -qr 1 --hidden
updated to hidden changeset a3571a6d8234
(hidden revision 'a3571a6d8234' was rewritten as: 0444ce380f11)
$ cd ..
Forcefully update the caches in each repo once; we're not using --debug since we
know that these are going to update some/all of the caches, we don't need to
know which.
$ hg -R orig debugupdatecache
$ hg -R other debugupdatecache
The branchheads caches should not change just because we're accessing from a
different repo each time. If they were stale, we'd get another line in the
output per filtername that's stale.
$ hg -R orig debugupdatecache --debug
updating the branch cache
$ hg -R other debugupdatecache --debug
updating the branch cache
$ hg -R orig debugupdatecache --debug
updating the branch cache
$ hg -R other debugupdatecache --debug
updating the branch cache