| Submitter | Pierre-Yves David |
|---|---|
| Date | March 28, 2017, 12:03 p.m. |
| Message ID | <a710d3c24acd544408fe.1490702624@nodosa.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/19780/ |
| State | Accepted |
| Headers | show |
Comments
On 3/28/17 1:03 PM, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> > # Date 1490673691 -7200 > # Tue Mar 28 06:01:31 2017 +0200 > # Node ID a710d3c24acd544408fe77243102325514f5f697 > # Parent e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9 > # EXP-Topic tags > tags: extract fnode retrieval in its own function s/in/into Other than this nitpick (which I missed the first time), this whole series looks good to me and is a nice clean-up. Marked at pre-reviewed. > My main goal here is to be able to reuse this logic easily. As a side effect > this important logic is now insulated and the code is clearer. > > diff --git a/mercurial/tags.py b/mercurial/tags.py > --- a/mercurial/tags.py > +++ b/mercurial/tags.py > @@ -341,15 +341,27 @@ def _readtagcache(ui, repo): > # potentially expensive search. > return ([], {}, valid, None, True) > > - starttime = util.timer() > > # 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. > + cachefnode = _getfnodes(ui, repo, repoheads) > + > + # Caller has to iterate over all heads, but can use the filenodes in > + # cachefnode to get to each .hgtags revision quickly. > + return (repoheads, cachefnode, valid, None, True) > + > +def _getfnodes(ui, repo, nodes): > + """return .hgtags fnodes for a list of changeset nodes > + > + Return value is a {node: fnode} mapping. There will be no entry for nodes > + without a '.hgtags' file. > + """ > + starttime = util.timer() > fnodescache = hgtagsfnodescache(repo.unfiltered()) > cachefnode = {} > - for head in reversed(repoheads): > + for head in reversed(nodes): > fnode = fnodescache.getfnode(head) > if fnode != nullid: > cachefnode[head] = fnode > @@ -361,10 +373,7 @@ def _readtagcache(ui, repo): > '%d/%d cache hits/lookups in %0.4f ' > 'seconds\n', > fnodescache.hitcount, fnodescache.lookupcount, 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, valid, None, True) > + return cachefnode > > def _writetagcache(ui, repo, valid, cachetags): > filename = _filename(repo) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=9oe9NoqznrBwRc0-yDKyoGJZtA90HiAagic3zdbDMpI&s=fyy1uiP9GxkbjMvmwkeaojB64TBVTNm8VNBNcNnwKnw&e=
On Tue, 28 Mar 2017 20:20:05 +0100, Ryan McElroy wrote: > On 3/28/17 1:03 PM, Pierre-Yves David wrote: > > # HG changeset patch > > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> > > # Date 1490673691 -7200 > > # Tue Mar 28 06:01:31 2017 +0200 > > # Node ID a710d3c24acd544408fe77243102325514f5f697 > > # Parent e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9 > > # EXP-Topic tags > > tags: extract fnode retrieval in its own function > s/in/into > > Other than this nitpick (which I missed the first time), this whole > series looks good to me and is a nice clean-up. Marked at pre-reviewed. Yeah, looks nice. Queued, thanks.
On Tue, Mar 28, 2017 at 5:03 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> > # Date 1490673691 -7200 > # Tue Mar 28 06:01:31 2017 +0200 > # Node ID a710d3c24acd544408fe77243102325514f5f697 > # Parent e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9 > # EXP-Topic tags > # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ > # hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r a710d3c24acd > tags: extract fnode retrieval in its own function > > My main goal here is to be able to reuse this logic easily. As a side effect > this important logic is now insulated and the code is clearer. > > diff --git a/mercurial/tags.py b/mercurial/tags.py > --- a/mercurial/tags.py > +++ b/mercurial/tags.py > @@ -341,15 +341,27 @@ def _readtagcache(ui, repo): > # potentially expensive search. > return ([], {}, valid, None, True) > > - starttime = util.timer() > > # 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. > + cachefnode = _getfnodes(ui, repo, repoheads) > + > + # Caller has to iterate over all heads, but can use the filenodes in > + # cachefnode to get to each .hgtags revision quickly. > + return (repoheads, cachefnode, valid, None, True) > + > +def _getfnodes(ui, repo, nodes): > + """return .hgtags fnodes for a list of changeset nodes > + > + Return value is a {node: fnode} mapping. There will be no entry for nodes > + without a '.hgtags' file. > + """ > + starttime = util.timer() > fnodescache = hgtagsfnodescache(repo.unfiltered()) > cachefnode = {} > - for head in reversed(repoheads): > + for head in reversed(nodes): "for head in nodes" is a little odd. I'll send a followup patch. > fnode = fnodescache.getfnode(head) > if fnode != nullid: > cachefnode[head] = fnode > @@ -361,10 +373,7 @@ def _readtagcache(ui, repo): > '%d/%d cache hits/lookups in %0.4f ' > 'seconds\n', > fnodescache.hitcount, fnodescache.lookupcount, 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, valid, None, True) > + return cachefnode > > def _writetagcache(ui, repo, valid, cachetags): > filename = _filename(repo) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/tags.py b/mercurial/tags.py --- a/mercurial/tags.py +++ b/mercurial/tags.py @@ -341,15 +341,27 @@ def _readtagcache(ui, repo): # potentially expensive search. return ([], {}, valid, None, True) - starttime = util.timer() # 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. + cachefnode = _getfnodes(ui, repo, repoheads) + + # Caller has to iterate over all heads, but can use the filenodes in + # cachefnode to get to each .hgtags revision quickly. + return (repoheads, cachefnode, valid, None, True) + +def _getfnodes(ui, repo, nodes): + """return .hgtags fnodes for a list of changeset nodes + + Return value is a {node: fnode} mapping. There will be no entry for nodes + without a '.hgtags' file. + """ + starttime = util.timer() fnodescache = hgtagsfnodescache(repo.unfiltered()) cachefnode = {} - for head in reversed(repoheads): + for head in reversed(nodes): fnode = fnodescache.getfnode(head) if fnode != nullid: cachefnode[head] = fnode @@ -361,10 +373,7 @@ def _readtagcache(ui, repo): '%d/%d cache hits/lookups in %0.4f ' 'seconds\n', fnodescache.hitcount, fnodescache.lookupcount, 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, valid, None, True) + return cachefnode def _writetagcache(ui, repo, valid, cachetags): filename = _filename(repo)