[PATCH 3 of 3] tags: preserve filtered .hgtags filenodes in tags cache (issue4550)

[Gregory Szorc]

On Wed, Feb 25, 2015 at 12:44 PM, Ryan McElroy <rm at fb.com> wrote:

>
> On 2/24/2015 1:11 AM, Gregory Szorc wrote:
>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1424769075 28800
>> #      Tue Feb 24 01:11:15 2015 -0800
>> # Branch stable
>> # Node ID 7f1904705c29ebe7de3874f2f03c42e261ed1c96
>> # Parent  7d72752b8da5bb2482e6eac47545a78ed3fff592
>> tags: preserve filtered .hgtags filenodes in tags cache (issue4550)
>>
>> If the tags cache is populated on an unfiltered repository and later
>> populated on a filtered repository, .hgtags filenode entries for
>> filtered revisions will disappear from the tags cache because the tags
>> cache code currently filters out filenode entries for revisions not
>> known to the current repo object. This behavior results in potentially
>> expensive recalculation of .hgtags filenode values for filtered
>> revisions. For evolution users, who create many hidden changesets and
>> heads, this could result in gradual slowdown, as each hidden head will
>> add overhead to resolving tags on an unfiltered repo.
>>
>> This patch makes the tags cache filtered revision aware. Filenode
>> entries for filtered revisions are preserved during reading and writing.
>> Entries are only dropped from the tags cache if they don't correspond to
>> a head, filtered or otherwise.
>>
>> diff --git a/mercurial/tags.py b/mercurial/tags.py
>> --- a/mercurial/tags.py
>> +++ b/mercurial/tags.py
>> @@ -246,12 +246,15 @@ def _readtagcache(ui, repo):
>>           return (None, None, tags, False)
>>       if cachefile:
>>           cachefile.close()               # ignore rest of file
>>   -    repoheads = repo.heads()
>> +    ourheads = repo.heads()
>> +    repo = repo.unfiltered()
>> +    allheads = repo.heads()
>> +
>>       # Case 2 (uncommon): empty repo; get out quickly and don't bother
>>       # writing an empty cache.
>> -    if repoheads == [nullid]:
>> +    if allheads == [nullid]:
>>           return ([], {}, {}, False)
>>         # Case 3 (uncommon): cache file missing or empty.
>>   @@ -268,14 +271,14 @@ 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 (ourheads, cachefnode, None, True)
>>         starttime = time.time()
>>         newheads = [head
>> -                for head in repoheads
>> +                for head in allheads
>>                   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
>> @@ -297,9 +300,9 @@ def _readtagcache(ui, repo):
>>              len(cachefnode), len(newheads), duration)
>>         # Caller has to iterate over all heads, but can use the filenodes
>> in
>>       # cachefnode to get to each .hgtags revision quickly.
>> -    return (repoheads, cachefnode, None, True)
>> +    return (ourheads, cachefnode, None, True)
>>     def _writetagcache(ui, repo, heads, tagfnode, cachetags):
>>         try:
>> @@ -309,29 +312,39 @@ def _writetagcache(ui, repo, heads, tagf
>>         ui.log('tagscache', 'writing tags cache file with %d heads and %d
>> tags\n',
>>               len(heads), len(cachetags))
>>   -    realheads = repo.heads()            # for sanity checks below
>> +    # We want to carry forward tagfnode entries that belong to filtered
>> revs,
>> +    # even if they aren't in the explicit list of heads. Since entries
>> in the
>> +    # cache must be in descending revlog order, we need to merge the sets
>> +    # before writing.
>> +    #
>> +    # When choosing what filenode entries to write, we must consider
>> both the
>> +    # filtered and unfiltered views. Otherwise, valid entries may be
>> dropped.
>> +    revs = {}
>> +    ourheads = set(repo.heads())
>> +    repo = repo.unfiltered()
>> +    unfilteredheads = set(repo.heads())
>> +    allheads = ourheads | unfilteredheads
>>
> /me confused: Why isn't unfilteredheads == allheads?
>

There are two flavors of DAG heads: unfiltered and filtered. If a true DAG
head is hidden, you'll need to record its parent, as it appears as a head
on filtered repos.

This is a case not explicitly tested by this patch series. This code may
even cause some heads to get dropped from the cache. However, that scenario
should be rare and will only result in loss of a handful of heads, not the
potentially hundreds that will be lost if all hidden DAG heads need
recomputed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150225/5df18b17/attachment.html>