Patchwork [1,of,5] tags: don't read .hgtags fnodes from tags cache files

login
register
mail settings
Submitter Gregory Szorc
Date April 16, 2015, 4:02 p.m.
Message ID <c56d6e53eb5f10b431f4.1429200124@gps-mbp.local>
Download mbox | patch
Permalink /patch/8717/
State Superseded
Commit d082c6ef9ec35bd8192bf3360df6c83b121adc05
Headers show

Comments

Gregory Szorc - April 16, 2015, 4:02 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1429193564 14400
#      Thu Apr 16 10:12:44 2015 -0400
# Node ID c56d6e53eb5f10b431f4e88b3959e063a0b665b6
# Parent  d5711c886d0b1acb2d18b92bf6e8ba6d9ad0c4b3
tags: don't read .hgtags fnodes from tags cache files

Now that we have a standalone and shared cache for .hgtags fnodes, the
.hgtags fnodes stored in the tags cache files are redundant.

Upcoming patches will change the format of the tags cache files to
remove this data and to improve the validation. To prepare for this, we
change the tags reading code to ignore all but the tip .hgtags fnodes
entries in existing tags cache files.

All fnodes lookups now go through our new shared cache, which is why
test output changed. Format of tags cache files has not yet changed.
Pierre-Yves David - April 16, 2015, 7:13 p.m.
On 04/16/2015 12:02 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1429193564 14400
> #      Thu Apr 16 10:12:44 2015 -0400
> # Node ID c56d6e53eb5f10b431f4e88b3959e063a0b665b6
> # Parent  d5711c886d0b1acb2d18b92bf6e8ba6d9ad0c4b3
> tags: don't read .hgtags fnodes from tags cache files

I've a couple of comment here. But I got them clarified with greg in 
real life. I've pushed a fixed version to the clowncopter. I'll 
patchbomb a V2 for reference.

> @@ -277,37 +277,34 @@ def _readtagcache(ui, repo):
>           cachelines = iter(cachefile)
>       except IOError:
>           cachefile = None
>
> -    cacherevs = []  # list of headrev
> -    cacheheads = [] # list of headnode
> -    cachefnode = {} # map headnode to filenode
> +    cachetiprev = None
> +    cachetipnode = None
>       if cachefile:
>           try:
> -            for line in cachelines:
> +            for i, line in enumerate(cachelines):
>                   if line == "\n":
>                       break
> +                if i != 0:
> +                    continue

What is this witchcraft about?


>       # Case 1 (common): tip is the same, so nothing has changed.
>       # (Unchanged tip trivially means no changesets have been added.
>       # But, thanks to localrepository.destroyed(), it also means none
>       # have been destroyed by strip or rollback.)
> -    if cacheheads and cacheheads[0] == tipnode and cacherevs[0] == tiprev:
> +    if cachetiprev is not None and cachetiprev == tiprev and \
> +            cachetipnode == tipnode:
>           tags = _readtags(ui, repo, cachelines, cachefile.name)
>           cachefile.close()
>           return (None, None, tags, False)
>       if cachefile:

do not use \ continuation, it makes kitten sad. wrap that in ()

Patch

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -277,37 +277,34 @@  def _readtagcache(ui, repo):
         cachelines = iter(cachefile)
     except IOError:
         cachefile = None
 
-    cacherevs = []  # list of headrev
-    cacheheads = [] # list of headnode
-    cachefnode = {} # map headnode to filenode
+    cachetiprev = None
+    cachetipnode = None
     if cachefile:
         try:
-            for line in cachelines:
+            for i, line in enumerate(cachelines):
                 if line == "\n":
                     break
+                if i != 0:
+                    continue
+
                 line = line.split()
-                cacherevs.append(int(line[0]))
-                headnode = bin(line[1])
-                cacheheads.append(headnode)
-                if len(line) == 3:
-                    fnode = bin(line[2])
-                    cachefnode[headnode] = fnode
+                cachetiprev = int(line[0])
+                cachetipnode = bin(line[1])
         except Exception:
-            # corruption of the tags cache, just recompute it
-            cacheheads = []
-            cacherevs = []
-            cachefnode = {}
+            # corruption of the cache, just recompute it.
+            pass
 
     tipnode = repo.changelog.tip()
     tiprev = len(repo.changelog) - 1
 
     # Case 1 (common): tip is the same, so nothing has changed.
     # (Unchanged tip trivially means no changesets have been added.
     # But, thanks to localrepository.destroyed(), it also means none
     # have been destroyed by strip or rollback.)
-    if cacheheads and cacheheads[0] == tipnode and cacherevs[0] == tiprev:
+    if cachetiprev is not None and cachetiprev == tiprev and \
+            cachetipnode == tipnode:
         tags = _readtags(ui, repo, cachelines, cachefile.name)
         cachefile.close()
         return (None, None, tags, False)
     if cachefile:
@@ -334,22 +331,19 @@  def _readtagcache(ui, repo):
     # exposed".
     if not len(repo.file('.hgtags')):
         # No tags have ever been committed, so we can avoid a
         # potentially expensive search.
-        return (repoheads, cachefnode, None, True)
+        return (repoheads, {}, None, True)
 
     starttime = time.time()
 
-    newheads = [head
-                for head in repoheads
-                if head not in set(cacheheads)]
-
     # Now we have to lookup the .hgtags filenode for every new head.
     # This is the most expensive part of finding tags, so performance
     # depends primarily on the size of newheads.  Worst case: no cache
     # file, so newheads == repoheads.
     fnodescache = hgtagsfnodescache(repo.unfiltered())
-    for head in reversed(newheads):
+    cachefnode = {}
+    for head in reversed(repoheads):
         fnode = fnodescache.getfnode(head)
         if fnode != nullid:
             cachefnode[head] = fnode
 
diff --git a/tests/test-tags.t b/tests/test-tags.t
--- a/tests/test-tags.t
+++ b/tests/test-tags.t
@@ -344,9 +344,9 @@  Extra junk data at the end should get ov
 
   $ hg blackbox -l 5
   1970/01/01 00:00:00 bob> tags
   1970/01/01 00:00:00 bob> writing 24 bytes to cache/hgtagsfnodes1
-  1970/01/01 00:00:00 bob> 0/1 cache hits/lookups in * seconds (glob)
+  1970/01/01 00:00:00 bob> 2/3 cache hits/lookups in * seconds (glob)
   1970/01/01 00:00:00 bob> writing tags cache file with 3 heads and 1 tags
   1970/01/01 00:00:00 bob> tags exited 0 after * seconds (glob)
 
 #if unix-permissions no-root
@@ -395,9 +395,9 @@  Stripping doesn't truncate the tags cach
   bar                                1:78391a272241
 
   $ hg blackbox -l 4
   1970/01/01 00:00:00 bob> tags
-  1970/01/01 00:00:00 bob> 1/1 cache hits/lookups in * seconds (glob)
+  1970/01/01 00:00:00 bob> 3/3 cache hits/lookups in * seconds (glob)
   1970/01/01 00:00:00 bob> writing tags cache file with 3 heads and 1 tags
   1970/01/01 00:00:00 bob> tags exited 0 after * seconds (glob)
 
   $ f --size .hg/cache/hgtagsfnodes1
@@ -412,9 +412,9 @@  Stripping doesn't truncate the tags cach
 
   $ hg blackbox -l 5
   1970/01/01 00:00:00 bob> tags
   1970/01/01 00:00:00 bob> writing 24 bytes to cache/hgtagsfnodes1
-  1970/01/01 00:00:00 bob> 0/1 cache hits/lookups in * seconds (glob)
+  1970/01/01 00:00:00 bob> 2/3 cache hits/lookups in * seconds (glob)
   1970/01/01 00:00:00 bob> writing tags cache file with 3 heads and 1 tags
   1970/01/01 00:00:00 bob> tags exited 0 after * seconds (glob)
   $ f --size .hg/cache/hgtagsfnodes1
   .hg/cache/hgtagsfnodes1: size=144