Patchwork [04,of,10,V2] branchmap: takes filtered revision in account for cache calculation

login
register
mail settings
Submitter Pierre-Yves David
Date Dec. 28, 2012, 12:56 a.m.
Message ID <13d786f9f076697469cb.1356656179@yamac.lan>
Download mbox | patch
Permalink /patch/309/
State Accepted
Commit c351759ab0a07dcd92d6312ab0a02e5bfaafde4a
Headers show

Comments

Pierre-Yves David - Dec. 28, 2012, 12:56 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
# Date 1356655623 -3600
# Node ID 13d786f9f076697469cbfe87becbac340eff17e0
# Parent  43650a9b9e09a53169ddd7e1535b816e470b3e5b
branchmap: takes filtered revision in account for cache calculation

Tracking tipnode and tiprev is not enough to ensure validaty of the cache as
they do not help distinguish a cache that ignored various revisions below
tiprev.

To detect such difference, we build a hash of all ignored revisions. This hash
is then used when checking the validity of a cache for a repo.
Dave S - Dec. 28, 2012, 5:44 p.m.
On Thu, Dec 27, 2012 at 4:55 PM, Pierre-Yves David
 <pierre-yves.david at ens-lyon.org> wrote:
 >
 > On 26 d?c. 2012, at 23:25, Dave S wrote:

>> "build a cache [...] and then return the hash of the cache to use when ..."
 >>
 >> My current thinking, such as it is, is that being explicit about the
 >> relationship of the hash to the cache is valuable.
 >
 > You got me totally confused here :-(
 >
 > Whats wrong? what do you suggest as an alternative ?

That was my alternative, but I'm satisfied with your taking Kevin's
 suggestion.

 There is still a small concern about "distinct", as I previously
 mentioned.  Do you mean "distinguish" (verb) or "distinct signature"
 (noun) ?

/dps
Augie Fackler - Dec. 31, 2012, 11:41 p.m.
On Dec 28, 2012, at 12:44 PM, Dave S <snidely.too at gmail.com> wrote:

> On Thu, Dec 27, 2012 at 4:55 PM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>> 
>> On 26 d?c. 2012, at 23:25, Dave S wrote:
> 
>>> "build a cache [...] and then return the hash of the cache to use when ..."
>>> 
>>> My current thinking, such as it is, is that being explicit about the
>>> relationship of the hash to the cache is valuable.
>> 
>> You got me totally confused here :-(
>> 
>> Whats wrong? what do you suggest as an alternative ?
> 
> That was my alternative, but I'm satisfied with your taking Kevin's
> suggestion.
> 
> There is still a small concern about "distinct", as I previously
> mentioned.  Do you mean "distinguish" (verb) or "distinct signature"
> (noun) ?

I'm going to stop reviewing this series here, since my MUA is totally failing to thread this and I can't find the conversation with Kevin. Pierre-Yves, can you resend patches 4:tip of this series when you're ready for re-review?

AF

> 
> /dps
> 
> -- 
> test signature -- please apply at front gate on Tuesdays only.
Pierre-Yves David - Dec. 31, 2012, 11:46 p.m.
On 1 janv. 2013, at 00:41, Augie Fackler wrote:

> 
> On Dec 28, 2012, at 12:44 PM, Dave S <snidely.too at gmail.com> wrote:
> 
>> On Thu, Dec 27, 2012 at 4:55 PM, Pierre-Yves David
>> <pierre-yves.david at ens-lyon.org> wrote:
>>> 
>>> On 26 d?c. 2012, at 23:25, Dave S wrote:
>> 
>>>> "build a cache [...] and then return the hash of the cache to use when ..."
>>>> 
>>>> My current thinking, such as it is, is that being explicit about the
>>>> relationship of the hash to the cache is valuable.
>>> 
>>> You got me totally confused here :-(
>>> 
>>> Whats wrong? what do you suggest as an alternative ?
>> 
>> That was my alternative, but I'm satisfied with your taking Kevin's
>> suggestion.
>> 
>> There is still a small concern about "distinct", as I previously
>> mentioned.  Do you mean "distinguish" (verb) or "distinct signature"
>> (noun) ?
> 
> I'm going to stop reviewing this series here, since my MUA is totally failing to thread this and I can't find the conversation with Kevin. Pierre-Yves, can you resend patches 4:tip of this series when you're ready for re-review?

V2 is the resend ready for re-review. Dave S feedback have been resolved as far I understand it.

Thanks for looking at it. This series if on the critical path for filtering

Patch

diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -5,10 +5,11 @@ 
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
 from node import bin, hex, nullid, nullrev
 import encoding
+import util
 
 def read(repo):
     try:
         f = repo.opener("cache/branchheads")
         lines = f.read().split('\n')
@@ -67,22 +68,45 @@  def updatecache(repo):
     repo._branchcache = partial
 
 class branchcache(dict):
     """A dict like object that hold branches heads cache"""
 
-    def __init__(self, entries=(), tipnode=nullid, tiprev=nullrev):
+    def __init__(self, entries=(), tipnode=nullid, tiprev=nullrev,
+                 filteredhash=None):
         super(branchcache, self).__init__(entries)
         self.tipnode = tipnode
         self.tiprev = tiprev
+        self.filteredhash = filteredhash
+
+    def _hashfiltered(self, repo):
+        """build hash of revision filtered in the current cache
+
+        Tracking tipnode and tiprev is not enough to ensure validaty of the
+        cache as they do not help to distinct cache that ignored various
+        revision bellow tiprev.
+
+        To detect such difference, we build a cache of all revision ignored."""
+        cl = repo.changelog
+        if not cl.filteredrevs:
+            return None
+        key = None
+        revs = sorted(r for r in cl.filteredrevs if r <= self.tiprev)
+        if revs:
+            s = util.sha1()
+            for rev in revs:
+                s.update('%s;' % rev)
+            key = s.digest()
+        return key
 
     def validfor(self, repo):
         """Is the cache content valide regarding a repo
 
         - False when cached tipnode are unknown or if we detect a strip.
         - True when cache is up to date or a subset of current repo."""
         try:
-            return self.tipnode == repo.changelog.node(self.tiprev)
+            return ((self.tipnode == repo.changelog.node(self.tiprev))
+                    and (self.filteredhash == self._hashfiltered(repo)))
         except IndexError:
             return False
 
 
     def write(self, repo):
@@ -170,5 +194,6 @@  class branchcache(dict):
             for heads in self.values():
                 tiprev = max(cl.rev(node) for node in heads)
                 if tiprev > self.tiprev:
                     self.tipnode = cl.node(tiprev)
                     self.tiprev = tiprev
+        self.filteredhash = self._hashfiltered(repo)