This is an archive of the discontinued Mercurial Phabricator instance.

Differential D462

undo: preview perf improvements
ClosedPublic

Authored by felixmerk on Aug 21 2017, 2:31 PM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGX2ed26f4a0c71: undo: preview perf improvements
Summary

Based on Adam Simpkin's ideas, faster code for the template functions.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

felixmerk created this revision.Aug 21 2017, 2:31 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 21 2017, 2:31 PM
quark accepted this revision.Aug 21 2017, 3:18 PM
quark added a subscriber: quark.

It's now binary nodes, so maybe hex should be removed. But I'm fine with doing that in a later patch.

This revision is now accepted and ready to land.Aug 21 2017, 3:18 PM
quark added inline comments.Aug 21 2017, 3:20 PM
hgext3rd/undo.py
549

It seems _oldmarks is just a hex node. In this case we can avoid creating a ctx object:

ctxhex = ctx.hex()
for ...
    if kv[1] == ctxhex:
quark added inline comments.Aug 21 2017, 3:24 PM
hgext3rd/undo.py
529

This could also be optimized to avoid create a ctx (repo[...]) object.

felixmerk updated this revision to Diff 1132.Aug 21 2017, 4:27 PM
Closed by commit rFBHGX2ed26f4a0c71: undo: preview perf improvements (authored by felixmerk). · Explain WhyAug 21 2017, 8:18 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext3rd/undo.py (20 lines)

Diff 1140

hgext3rd/undo.py

else: else:
querystring = revsetlang.formatspec('((::%ld) & draft())::', revs) querystring = revsetlang.formatspec('((::%ld) & draft())::', revs)
return subset & smartset.baseset(repo.revs(querystring)) return subset & smartset.baseset(repo.revs(querystring))
# Templates # Templates
templatefunc = registrar.templatefunc() templatefunc = registrar.templatefunc()
def _undonehexnodes(repo, reverseindex): def _undonehexnodes(repo, reverseindex):
repo = repo.unfiltered() revstring = revsetlang.formatspec('olddraft(0) - olddraft(%d)',
revstring = revsetlang.formatspec('draft() - olddraft(%d)', reverseindex) reverseindex)
revs = repo.revs(revstring) revs = repo.revs(revstring)
tonode = repo.changelog.node tonode = repo.changelog.node
hexnodes = [repo[tonode(x)] for x in revs] return [tonode(x) for x in revs]
return hexnodes
@templatefunc('undonecommits(reverseindex)') @templatefunc('undonecommits(reverseindex)')
def showundonecommits(context, mapping, args): def showundonecommits(context, mapping, args):
"""String. Changectxs added since reverseindex command.""" """String. Changectxs added since reverseindex command."""
reverseindex = templater.evalinteger(context, mapping, args[0], reverseindex = templater.evalinteger(context, mapping, args[0],
_('undonecommits needs an integer argument')) _('undonecommits needs an integer argument'))
repo = mapping['ctx']._repo repo = mapping['ctx']._repo
ctx = mapping['ctx'] ctx = mapping['ctx']
hexnodes = _undonehexnodes(repo, reverseindex) hexnodes = _undonehexnodes(repo, reverseindex)
if ctx in hexnodes: if ctx.node() in hexnodes:
result = ctx.hex() result = ctx.hex()
else: else:
result = None result = None
return result return result
def _donehexnodes(repo, reverseindex): def _donehexnodes(repo, reverseindex):
repo = repo.unfiltered() repo = repo.unfiltered()
revstring = revsetlang.formatspec('olddraft(%d)', reverseindex) revstring = revsetlang.formatspec('olddraft(%d)', reverseindex)
revs = repo.revs(revstring) revs = repo.revs(revstring)
tonode = repo.changelog.node tonode = repo.changelog.node
hexnodes = [repo[tonode(x)] for x in revs] return [tonode(x) for x in revs]
return hexnodes
@templatefunc('donecommits(reverseindex)') @templatefunc('donecommits(reverseindex)')
def showdonecommits(context, mapping, args): def showdonecommits(context, mapping, args):
"""String. Changectxs reverseindex repo states ago.""" """String. Changectxs reverseindex repo states ago."""
reverseindex = templater.evalinteger(context, mapping, args[0], reverseindex = templater.evalinteger(context, mapping, args[0],
_('donecommits needs an integer argument')) _('donecommits needs an integer argument'))
repo = mapping['ctx']._repo repo = mapping['ctx']._repo
ctx = mapping['ctx'] ctx = mapping['ctx']
hexnodes = _donehexnodes(repo, reverseindex) hexnodes = _donehexnodes(repo, reverseindex)
if ctx in hexnodes: if ctx.node() in hexnodes:
result = ctx.hex() result = ctx.hex()
else: else:
result = None result = None
return result return result
def _oldmarks(repo, reverseindex): def _oldmarks(repo, reverseindex):
nodedict = _readindex(repo, reverseindex) nodedict = _readindex(repo, reverseindex)
bookstring = _readnode(repo, "bookmarks.i", nodedict["bookmarks"]) bookstring = _readnode(repo, "bookmarks.i", nodedict["bookmarks"])
"""List of Strings. Bookmarks that used to be at the changectx reverseindex """List of Strings. Bookmarks that used to be at the changectx reverseindex
repo states ago.""" repo states ago."""
reverseindex = templater.evalinteger(context, mapping, args[0], reverseindex = templater.evalinteger(context, mapping, args[0],
_('oldbookmarks needs an integer argument')) _('oldbookmarks needs an integer argument'))
repo = mapping['ctx']._repo repo = mapping['ctx']._repo
ctx = mapping['ctx'] ctx = mapping['ctx']
oldmarks = _oldmarks(repo, reverseindex) oldmarks = _oldmarks(repo, reverseindex)
bookmarks = [] bookmarks = []
ctxhex = ctx.hex()
for kv in oldmarks: for kv in oldmarks:
if repo[kv[1]] == repo[ctx]: if kv[1] == ctxhex:
quarkUnsubmitted
Not Done

This could also be optimized to avoid create a ctx (repo[...]) object.

quark: This could also be optimized to avoid create a `ctx` (`repo[...]`) object.
bookmarks.append(kv[0]) bookmarks.append(kv[0])
active = repo._activebookmark active = repo._activebookmark
makemap = lambda v: {'bookmark': v, 'active': active, 'current': active} makemap = lambda v: {'bookmark': v, 'active': active, 'current': active}
f = templatekw._showlist('bookmark', bookmarks, mapping) f = templatekw._showlist('bookmark', bookmarks, mapping)
return templatekw._hybrid(f, bookmarks, makemap, lambda x: x['bookmark']) return templatekw._hybrid(f, bookmarks, makemap, lambda x: x['bookmark'])
@templatefunc('removedbookmarks(reverseindex)') @templatefunc('removedbookmarks(reverseindex)')
def removedbookmarks(context, mapping, args): def removedbookmarks(context, mapping, args):
"""List of Strings. Bookmarks that have been moved or removed from a given """List of Strings. Bookmarks that have been moved or removed from a given
changectx by reverseindex repo state.""" changectx by reverseindex repo state."""
reverseindex = templater.evalinteger(context, mapping, args[0], reverseindex = templater.evalinteger(context, mapping, args[0],
_('removedbookmarks needs an integer argument')) _('removedbookmarks needs an integer argument'))
repo = mapping['ctx']._repo repo = mapping['ctx']._repo
ctx = mapping['ctx'] ctx = mapping['ctx']
currentbookmarks = mapping['ctx'].bookmarks() currentbookmarks = mapping['ctx'].bookmarks()
oldmarks = _oldmarks(repo, reverseindex) oldmarks = _oldmarks(repo, reverseindex)
oldbookmarks = [] oldbookmarks = []
ctxhex = ctx.hex()
for kv in oldmarks: for kv in oldmarks:
if repo[kv[1]] == repo[ctx]: if kv[1] == ctxhex:
quarkUnsubmitted
Not Done

It seems _oldmarks is just a hex node. In this case we can avoid creating a ctx object:

ctxhex = ctx.hex()
for ...
    if kv[1] == ctxhex:
quark: It seems `_oldmarks` is just a hex node. In this case we can avoid creating a `ctx` object…
oldbookmarks.append(kv[0]) oldbookmarks.append(kv[0])
bookmarks = list(set(currentbookmarks) - set(oldbookmarks)) bookmarks = list(set(currentbookmarks) - set(oldbookmarks))
active = repo._activebookmark active = repo._activebookmark
makemap = lambda v: {'bookmark': v, 'active': active, 'current': active} makemap = lambda v: {'bookmark': v, 'active': active, 'current': active}
f = templatekw._showlist('bookmark', bookmarks, mapping) f = templatekw._showlist('bookmark', bookmarks, mapping)
return templatekw._hybrid(f, bookmarks, makemap, lambda x: x['bookmark']) return templatekw._hybrid(f, bookmarks, makemap, lambda x: x['bookmark'])
# Undo: # Undo: