# HG changeset patch # User Pierre-Yves David # Date 1493684013 -7200 # Node ID 01efebff13ece0cef63c9690e9f199aa4fce4cb5 # Parent 268970463144fbe78b1f263ca02308a967579cd8 obscache: skip the cache entirely if not up to date The current update code has some race condition windows around updating. But we also ensure the cache are up to date with every transaction. So outdated cache mean another client have been mudding the repository but things will get back in place at the next transaction. So we just skip using the cache when not up to date. This is not the best we could do. But this is good enough for now. diff -r 268970463144 -r 01efebff13ec hgext3rd/evolve/obscache.py --- a/hgext3rd/evolve/obscache.py Mon May 01 15:49:36 2017 +0200 +++ b/hgext3rd/evolve/obscache.py Tue May 02 02:13:33 2017 +0200 @@ -118,7 +118,7 @@ # key in all case because we want to skip reading the obsstore content. We # could have a smarter implementation here. # - # In pratice the cache should be updated after each transaction within a + # In pratice the cache is only updated after each transaction within a # lock. So we should be fine. We could enforce this with a new repository # requirement (or fix the race, that is not too hard). invalid = (False, 0, 0) @@ -211,6 +211,10 @@ self._cachekey = None self._data = bytearray() + def uptodate(self, repo): + valid, startrev, startidx = upgradeneeded(repo, self._cachekey) + return (valid and startrev is None and startidx is None) + def update(self, repo): """Iteratively update the cache with new repository data""" # If we do not have any data, try loading from disk @@ -275,9 +279,10 @@ if r is not None and (startrev is None or r < startrev): self._data[r] = 1 - # XXX note that there are a race condition here, since the repo "might" - # have changed side the cache update above. However, this code will - # mostly be running in a lock so we ignore the issue for now. + assert repo._currentlock(repo._lockref) is not None + # XXX note that there are a potential race condition here, since the + # repo "might" have changed side the cache update above. However, this + # code will only be running in a lock so we ignore the issue for now. # # To work around this, 'upgradeneeded' should return a bounded amount # of changeset and markers to read with their associated cachekey. see @@ -308,14 +313,26 @@ self._cachekey = struct.unpack(self._headerformat, data[:headersize]) self._data = bytearray(data[headersize:]) -def _computeobsoleteset(repo): +def _computeobsoleteset(orig, repo): """the set of obsolete revisions""" obs = set() + repo = repo.unfiltered() notpublic = repo._phasecache.getrevset(repo, (phases.draft, phases.secret)) if notpublic: - repo = repo.unfiltered() obscache = repo.obsstore.obscache - obscache.update(repo) + # Since we warm the cache at the end of every transaction, the cache + # should be up to date. However a non-enabled client might have touced + # the repository. + # + # Updating the cache without a lock is sloppy, so we fallback to the + # old method and rely on the fact the next transaction will write the + # cache down anyway. + # + # With the current implementation updating the cache will requires to + # load the obsstore anyway. Once loaded, hitting the obsstore directly + # will be about as fast.. + if not obscache.uptodate(repo): + return orig(repo) isobs = obscache.get for r in notpublic: if isobs(r): @@ -324,7 +341,9 @@ @eh.uisetup def cachefuncs(ui): - obsolete.cachefuncs['obsolete'] = _computeobsoleteset + orig = obsolete.cachefuncs['obsolete'] + wrapped = lambda repo: _computeobsoleteset(orig, repo) + obsolete.cachefuncs['obsolete'] = wrapped @eh.reposetup def setupcache(ui, repo):