changeset 2300:01efebff13ec

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.

--- 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):