revset: drop "draft() & ::x" optimization for better performance

Summary:
The optimization was useful for revlog backend. But with segmented changelog
backend the optimization can actually slow things down. Let's drop it.

Before:

  In [1]: repo.revs('not public() & ::cf269d')
  Out[1]: <generatorset+>

  In [3]: %timeit list(repo.revs('not public() & ::cf269d'))
  829 µs ± 16.9 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

After:

  In [1]: repo.revs('not public() & ::cf269d')
  Out[1]: <nameset+ <spans [415dd9a55c7420f9405f9b6a9fbe1690e43e3633:cf269d03b5c93d73c1af324bf29cddb8ef72bb17+N7095:N7110]>>

  In [3]: %timeit list(repo.revs('not public() & ::cf269d'))
  1000 loops, best of 3: 550 µs per loop

This is found by user report of slow `cloud sync` with the following stack in sigtrace:

  File "edenscm/hgext/commitcloud/commands.py", line 1317, in cloudsync
    ret = sync.sync(
  File "edenscm/hgext/commitcloud/sync.py", line 108, in sync
    rc, synced = _sync(repo, *args, **kwargs)
  File "edenscm/hgext/commitcloud/sync.py", line 216, in _sync
    state.update(uploaded)
    # uploaded = [bin('8ae4a4b89ed421543bf1b9d14c5f5f9e479b1fa6'), bin('62762022d626f23413de7bb2bf9291ed0475e4e5'), bin('50f3ff4dd6f756be55a550905890b8ce49be8233'), bin('100ef8fda520e086156c0a494f8fa8f87d8c7e7d'), bin('a99e9e174db4647e7e77bb998e2ec472d4d96185'), bin('c287b11215b0c3b595a651dd3c5a534d4a428646'), bin('58225ace5b20c3af7a5a817af919a75a40f67ab1'), bin('b468e1480bdae5dab886a59e230813520b3230e8'), ...]
  File "edenscm/hgext/commitcloud/backupstate.py", line 122, in update
    self.heads = list(
  File "edenscm/mercurial/localrepo.py", line 1427, in nodes
    for r in self.revs(expr, *args):
    # args = ({bin('037634b9cc756f9770bf70bcb940278caa65711f'), bin('06f2a41fc8ff68392ba7433832c1e585c2f776dd'), bin('07ad90c39273a46e783cc5f5a281f2a20cc0e77c'), bin('088a22ab414080df17ce35a35001663699f5f994'), bin('089073c8e0234e403fa1b95d4780eb80269a020c'), bin('09c7593665e19aaa98414e7767601f77794b9373'), bin('0d99d86f7699482949bd0b0611475c79455dea64'), bin('100ef8fda520e086156c0a494f8fa8f87d8c7e7d'), ...}, [bin('8ae4a4b89ed421543bf1b9d14c5f5f9e479b1fa6'), bin('62762022d626f23413de7bb2bf9291ed0475e4e5'), bin('50f3ff4dd6f756be55a550905890b8ce49be8233'), bin('100ef8fda520e086156c0a494f8fa8f87d8c7e7d'), bin('a99e9e174db4647e7e77bb998e2ec472d4d96185'), bin('c287b11215b0c3b595a651dd3c5a534d4a428646'), bin('58225ace5b20c3af7a5a817af919a75a40f67ab1'), bin('b468e1480bdae5dab886a59e230813520b3230e8'), ...])
    # expr = 'heads((not public() & ::%ln) + (not public() & ::%ln))'
  File "edenscm/mercurial/localrepo.py", line 1404, in revs
    return m(self, subset=subset)
  File "edenscm/mercurial/revset.py", line 2799, in mfunc
    return getset(repo, subset, tree, order)
    # order = 'define'
    # subset = <fullreposet+>
    # tree = ('func', ('symbol', 'heads'), ('or', (...., ...., ....)))
  File "edenscm/mercurial/revset.py", line 105, in getset
    return methods[x[0]](repo, subset, *x[1:], order=order)
    # order = 'define'
    # subset = <fullreposet+>
    # x = ('func', ('symbol', 'heads'), ('or', (...., ...., ....)))
  File "edenscm/mercurial/revset.py", line 359, in func
    return func(repo, subset, b)
    # b = ('or', ('list', (...., ...., ....), (...., ...., ....)))
    # subset = <fullreposet+>
  File "edenscm/mercurial/revset.py", line 1495, in heads
    return subset & cl.torevset(cl.dag.heads(cl.tonodes(s)))
    # s = <addset- <generatorset+>, <generatorset+>>
    # subset = <fullreposet+>
  File "edenscm/mercurial/changelog2.py", line 257, in tonodes
    return self.inner.tonodes(revs)
    # revs = <addset- <generatorset+>, <generatorset+>>
  File "edenscm/mercurial/smartset.py", line 1187, in _iterordered
    for val in it:
    # val = 72057594038088038
  File "edenscm/mercurial/dagop.py", line 116, in _walkrevtree
    for prev in pfunc(currev):
    # currev = 72057594038088407
    # prev = -1
  File "edenscm/mercurial/dagop.py", line 175, in plainpfunc
    return cl.parentrevs(rev)[:cut]
    # rev = 72057594038098023
  File "edenscm/mercurial/changelog2.py", line 639, in parentrevs
    return list(map(self.rev, self.parents(self.node(rev))))
    # rev = 72057594038098023
  File "edenscm/mercurial/changelog2.py", line 636, in parents
    return parents
    # parents = [bin('13b365b8e4f052ee291bacd8ef89a66bde19e066'), bin('0000000000000000000000000000000000000000')]

According to the reporter, the change makes `cloud sync` faster.

Reviewed By: DurhamG

Differential Revision: D32804901

fbshipit-source-id: 9498978434cb27f2f09f243d8afb9b5ebe26bfd9
This commit is contained in:
Jun Wu 2021-12-03 17:00:12 -08:00 committed by Facebook GitHub Bot
parent ef0f7fc425
commit a2f122678a
3 changed files with 15 additions and 15 deletions

View File

@ -125,13 +125,14 @@ def crdump(ui, repo, *revs, **opts):
except KeyError:
pass # lfs extension is not enabled
notbackedup = set(repo[rev].node() for rev in revs)
# notbackedup is a revset
notbackedup = revs
if ui.configbool("crdump", "commitcloud", False):
try:
oldquiet = repo.ui.quiet
# Silence any output from commitcloud
repo.ui.quiet = True
notbackedup = commitcloud.backup.backup(repo, revs)[1]
_backedup, notbackedup = commitcloud.backup.backup(repo, revs)
except Exception:
# Don't let commit cloud exceptions block crdump
pass
@ -148,7 +149,7 @@ def crdump(ui, repo, *revs, **opts):
"p1": {"node": ctx.p1().hex()},
"user": encoding.fromlocal(ctx.user()),
"bookmarks": list(map(encoding.fromlocal, ctx.bookmarks())),
"commit_cloud": False if ctx.node() in notbackedup else True,
"commit_cloud": False if ctx.rev() in notbackedup else True,
"manifest_node": hex(ctx.manifestnode()),
}
if ctx.p1().phase() != phases.public:

View File

@ -398,11 +398,6 @@ def _optimize(x):
wb, tb = _optimize(x[2])
w = min(wa, wb)
# (draft/secret/_notpublic() & ::x) have a fast path
m = _match("_() & ancestors(_)", ("and", ta, tb))
if m and getsymbol(m[1]) in {"draft", "secret", "_notpublic"}:
return w, _build("_phaseandancestors(_, _)", m[1], m[2])
# (::x and not ::y)/(not ::y and ::x) have a fast path
m = _matchonly(ta, tb) or _matchonly(tb, ta)
if m:

View File

@ -1686,7 +1686,7 @@ Test obsstore related revsets
D
G
Test `draft() & ::x` optimization
Test `draft() & ::x` is not optimized to _phaseandancestors:
$ hg init $TESTTMP/repo2
$ cd $TESTTMP/repo2
@ -1746,10 +1746,12 @@ Test `draft() & ::x` optimization
(symbol 'D3')))
(symbol 'S2')))))
* optimized:
(func
(symbol '_phaseandancestors')
(list
(and
(func
(symbol 'draft')
None)
(func
(symbol 'ancestors')
(or
(list
(difference
@ -1768,10 +1770,12 @@ Test `draft() & ::x` optimization
(symbol 'ancestors')
(symbol '9')))
* optimized:
(func
(symbol '_phaseandancestors')
(list
(and
(func
(symbol 'secret')
None)
(func
(symbol 'ancestors')
(symbol '9')))
$ hg debugrevspec --verify -p optimized '(not public()) & ancestors(S1+D2+P5, 1)'
* optimized: