They will be commonly used by revset and templater. It isn't easy to understand
how _expand() works, so I'll add comments by a follow-up patch.
The local variable 'alias' is renamed to 'a' to avoid shadowing the global
'alias' class.
It was odd that the revsetalias did the whole parsing stuff in __init__().
Instead, this patch adds a factory function to the aliasrules class, and
makes the alias (= revsetalias) class a plain-old value object.
We no longer need separate parsers. Only difference between _parsealiasdecl()
and _parsealiasdefn() is whether or not to flatten 'or' tree. Since alias
declaration should have no 'or' operator, there was no practical difference.
The original _parsealiasdefn() function is split into common _builddefn()
and revset-specific _parsealiasdefn(). revset._relabelaliasargs() is removed
as it is no longer used.
The doctests are ported by using the dummy parse().
The original _parsealiasdecl() function is split into common _builddecl()
and revset-specific _parsealiasdecl(). And the original _parsealiasdecl()
call is temporarily replaced by rules._builddecl(), which should be eliminated
later.
The doctests are mostly ported by using the dummy parse(), but the test for
'foo bar' is kept in _parsealiasdecl() as it checks if "pos != len(decl)" is
working. Also, 'foo($1)' test is added to make sure the alias tokenizer can
handle '$1' symbol, which is the only reason why we need _parsealiasdecl().
This class will keep syntax rules that are necessary to parse and expand
aliases. The implementations will be extracted from the revset module. In
order to make the porting easier, this class keeps parsedecl and parsedefn
separately, which will be unified later. Also, getlist and funcnode will
be refactored by future patches for better handling of the template aliases.
The following public functions will be added:
aliasrules.build(decl, defn) -> aliasobj
parse decl and defn into an object that keeps alias name, arguments
and replacement tree.
aliasrules.buildmap(aliasitems) -> aliasdict
helper to build() a dict of alias objects from a list of (decl, defn)
aliasrules.expand(aliasdict, tree) -> tree
expand aliases in tree recursively
Because these functions aren't introduced by this series, there would remain
a few wrapper functions in the revset module. These ugly wrappers should be
eliminated by the next series.
This class is considered an inheritable namespace, which will host only
class/static methods. That's because it won't have no object-scope variables.
I'm not a big fan of using class as a syntax sugar, but I admit it can improve
code readability at some level. So let's give it a try.
It is possible to initialize a baseset directly from a set object. However, in
this case the iteration order was inherited from the set. Set have undefined
iteration order (especially cpython and pypy will have different one) so we
should not rely on it anywhere.
Therefor we declare the baseset "ascending" to enforce a consistent iteration
order. The sorting is done lazily by the baseset class and should have no
performance impact when it does not matter.
This makes test-revset.t pass with pypy.
Cpython and pypy have different way to build and order set, so the result of
list(myset) is different. We work around this by using the sorted version of the
data when displaying a list.
This get pypy closer to pass test-revset.t.
PyPy would sometime call __len__ at points where it things preallocating
the container makes sense. Change the doctests so they're using generator
expressions and not list comprehensions
Since I'm going to extract a common alias parser, I want to eliminate
dependencies to the revset parsing rules. These functions are trivial,
so we can go without them.
If tree is a tuple, it must have at least one element. Also the length of node
tuple is guaranteed by the syntax elements. (e.g. 'func' must have 3 items.)
This change will help inlining these trivial functions in future patches.
Since _parsealiasdefn() rejects unknown alias arguments, _checkaliasarg() is
unnecessary. New test is added to make sure unknown '$n' symbols are rejected.
In short, this patch moves the hack from tokenizedefn() to _relabelaliasargs(),
which is called after parsing. This change aims to eliminate tight dependency
on the revset tokenizer.
Before this patch, we had to rewrite an alias argument to a pseudo function:
"$1" -> "_aliasarg('$1')"
('symbol', '$1') -> ('function', ('symbol', '_aliasarg'), ('string', '$1'))
This was because the tokenizer must generate tokens that are syntactically
valid. By moving the process to the parsing phase, we can assign a unique tag
to an alias argument.
('symbol', '$1') -> ('_aliasarg', '$1')
Since new _aliasarg node never be generated from a user input, we no longer
have to verify a user input at findaliases(). The test for _aliasarg("$1") is
removed as it is syntactically valid and should pass the parsing phase.
Previous patch makes this classes useless by replacing it with
revsetpredicate of registrar.
BTW, extpredicate itself has already been broken by that patch,
because revsetpredicate of registrar doesn't have compatibility with
original predicate (derived from funcregistrar of registrar), in fact.
A filteredset is heavily used, but it cannot provide a printable information
how given set is filtered because a condition is an arbitrary callable object.
This patch adds an optional "condrepr" object that is used only by repr(). To
minimize the maintaining/runtime overhead of "condrepr", its type is overloaded
as follows:
type example
-------- ---------------------------------
tuple ('<not %r>', other)
str '<branch closed>'
callable lambda: '<branch %r>' % sorted(b)
object other
To make all built-in predicates be known to hggettext, loading
built-in predicates by loadpredicate() should be placed before fixing
i18nfunctions but after all of predicate decorating.
revsetpredicate is used to replace revset.predicate and
revset.extpredicate in subsequent patches.
This patch also adds loadpredicate() to revset, because this
combination helps to figure out how the name of safe predicate is put
into safesymbols.
This patch still uses safesymbols set to examine whether the predicate
corresponded to the 'name' is safe from DoS attack or not, because
just setting func._safe property needs changes below for such
examination.
before:
name in revset.safesymbols
after:
getattr(revset.symbols.get(name, None), '_safe', False)
"automatic registration" described in help doc of revsetpredicate
class will be achieved by the subsequent patch, which lists
loadpredicate() up in dispatch.extraloaders.
It's a source of UnboundLocalError to define and use local variables
conditionally. As getstring() always returns a str, "pat" can be initialized
to None.
Previously, revsets like 'X - Y' were translated to be 'X and not Y'. This can
be expensive, since if Y is a single commit then 'not Y' becomes a huge set and
sometimes the query optimizer doesn't account for it well.
This patch changes revsets to use the built in smartset minus operator, which is
often smarter than 'X and not Y'.
On a large repo this saves 2.2 seconds on rebase and histedit because "X:: - X"
becomes almost instant.
Relevant performance numbers from revsetbenchmark.py
revset #13: roots((tip~100::) - (tip~100::tip))
plain min max first last reverse rev..rst rev..ast sort sor..rst sor..ast
0) 0.001080 0.001107 0.001102 0.001118 0.001121 0.001114 0.001141 0.001123 0.001099 0.001123 0.001137
1) 0.000708 65% 0.000738 66% 0.000735 66% 0.000739 66% 0.000784 69% 0.000780 70% 0.000807 70% 0.000756 67% 0.000727 66% 0.000759 67% 0.000808 71%
revset #14: roots((0::) - (0::tip))
plain min max first last reverse rev..rst rev..ast sort sor..rst sor..ast
0) 0.131304 0.079168 0.133129 0.076560 0.048179 0.133349 0.049153 0.077097 0.129689 0.076212 0.048543
1) 0.065066 49% 0.036941 46% 0.066063 49% 0.034755 45% 0.048558 0.071091 53% 0.047679 0.034984 45% 0.064572 49% 0.035680 46% 0.048508
revset #22: (not public() - obsolete())
plain min max first last reverse rev..rst rev..ast sort sor..rst sor..ast
0) 0.000139 0.000133 0.000133 0.000138 0.000134 0.000155 0.000157 0.000152 0.000157 0.000156 0.000153
1) 0.000108 77% 0.000129 0.000129 0.000134 0.000132 0.000127 81% 0.000151 0.000147 0.000127 80% 0.000152 0.000149
revset #25: (20000::) - (20000)
plain min max first last reverse rev..rst rev..ast sort sor..rst sor..ast
0) 0.050560 0.045513 0.022593 0.043588 0.021909 0.045517 0.021822 0.044660 0.049740 0.044227 0.021819
1) 0.018614 36% 0.000171 0% 0.019659 87% 0.000168 0% 0.015543 70% 0.021069 46% 0.015623 71% 0.000180 0% 0.018658 37% 0.000186 0% 0.015750 72%
We can now specify from where the merge is performed. The experimental revset
is updated to take revisions as argument, allowing to test the feature.
This will become very useful for pick the 'rebase' default destination. For this
reason, we also exclude all descendants from the rebased set from the candidate
destinations. This descendants exclusion was not necessary for merge as default
destination would not be picked from anything else than a head.
I'm not super excited with the current error messages, but I would prefer to
delay an overall messages rework once 'hg rebase' is done getting a default
destination aligned with 'hg merge'.
Internal _matchfiles() function can take bunch of arguments, which would
lead to a maximum recursion depth error. This patch avoids the excessive
stack use by flattening 'list' nodes beforehand.
Since getlist() no longer takes a nested 'list' nodes, _parsealiasdecl()
also needs to flatten argument list, "aliasname($1, $2, ...)".
On repos with lots of heads, the filelog() code could spend several
minutes decompressing manifests. This change instead tries to
efficiently scan the changelog for candidates and decompress as few
manifests as possible. This is a regression introduced in 3.3 by the
linkrev adjustment code. Prior to that, filelog was nearly instant.
For the repo in the bug report, this improves time of a simple log
command from ~3 minutes to ~.5 seconds, a 360x speedup.
For the main Mercurial repo, a log of commands.py slows down from
1.14s to 1.45s, a 27% slowdown. This is still faster than the file()
revset, which takes 2.1 seconds.
The old _follow revset iterated over every file in the commit and checked if it
matched. For repos with large manifests, this could take 500ms. By switching to
use manifest.matches() we can take advantage of the fastpaths built in to
manifest.py that allows iterating over only the files in the matcher when it's a
simple matcher. This brings the time spent down from 500ms to 0ms during simple
operations like 'hg log -f file.txt'.
Using decorator can localize changes for adding (or removing) a "safe"
revset predicate function in source code.
To avoid accidentaly treating unsuitable predicates as safe, this
patch uses False as default value of "safe" argument. This forces safe
predicates to be decorated with explicit 'safe=True'.
Previous patch introduced 'revset.predicate' decorator to register
revset predicate function easily.
But it shouldn't be used in extension directly, because it registers
specified function immediately. Registration itself can't be restored,
even if extension loading fails after that.
Therefore, registration should be delayed until 'uisetup()' or so.
This patch uses 'extpredicate' decorator derived from 'delayregistrar'
to register predicate in extension easily.
This patch also tests whether 'registrar.delayregistrar' avoids
function registration if 'setup()' isn't invoked on it, because
'extpredicate' is the first user of it.
Using decorator can localize changes for adding (or removing) a revset
predicate function in source code.
It is also useful to pick predicates up for specific purpose. For
example, subsequent patch marks predicates as "safe" by decorator.
This patch defines 'parsefuncdecl()' in 'funcregistrar' class, because
this implementation can be uesd by other decorator class for fileset
predicate and template function.
This patch makes hg log <file|folder> faster by using changelog.readfiles
instead of changelog.read.
On our large repos for hg log <file|folder> -l5 operations that were taking:
- ~8s I see a 25% improvement
- ~15s, I see a 35% improvement
For recently modified folder/file, the difference is negligible as we don't
have to consider many revisions.
File matching is done by applying the matcher to all elements in the 'file'
field of all changesets in the repository. This requires to read/parse all
changesets in the repository and do a lot of matching. However about 1/3 of the
time of the function is used to create 'changectx' object and retrieve their
'file' field.
This is far too much overhead so we are skipping the changectx layer and
directly access the data from the changelog. This provide use significant speed
up:
repository: mozilla central 252524 revisions
command: hg perfrevset '_matchfiles("p:browser")'
Before: 15.899687s
After: 10.011705s
Slowdown is even more significant if you have a lot of namespace that slowdown
lookup.
The time is now spent with this approximate repartition:
Matcher: 20%
regexp matching: 10%
changelog.read: 80%
reading revision: 60%
checking hash: 15%
decompression: 15%
reading chunk: 30%
changelog parsing: 20%
decoding to local: 10%
The next easy win is probably to have more of the changelog stack implemented
using the CPython api.
Function in destutil are much simpler to wrap and more flexible than revset.
This also help consistency as 'destupdate' live here and cannot become a pure
revset anyway.
The revset is not ready for prime time yet. However it is useful to have some
version of it exposed to help candidate users to play with it and provide
feedback on what we should aim at.
We add a small test to make sure the code runs.
It's common for GUI or web frontend to fetch chunk of revisions per batch
size. Previously it was possible only if revisions were sorted by revision
number.
$ hg log -r 'limit({revspec} & :{last_known}, 101)'
So this patch introduces a general way to retrieve chunk of revisions after
skipping offset revisions.
$ hg log -r 'limit({revspec}, 100, {last_count})'
This is a dumb implementation. We can optimize it for baseset and spanset
later.
The home of 'Abort' is 'error' not 'util' however, a lot of code seems to be
confused about that and gives all the credit to 'util' instead of the
hardworking 'error'. In a spirit of equity, we break the cycle of injustice and
give back to 'error' the respect it deserves. And screw that 'util' poser.
For great justice.
We ultimately want this to be accessible through a revset, but there is too
much complexity here for that to work. Especially we'll have to return more
than just the destination to control the behavior (eg: bookmarks to activate,
etc).
To prevent cycle, a new module is created, it will receive other
destination/behavior function in the future.
Previously, calling 'if foo:' on a ordered filtered set would start iterating in
whatever the current direction was and return if a value was available. If the
current direction was ascending, but the set had a fastdesc available, this
meant we did a lot more work than necessary.
If this was applied without my previous max/min fixes, it would improve max()
performance (this was my first attempt at fixing the issue). Since those
previous fixes went in though, this doesn't have a visible benefit in the
benchmarks, but it does seem clearly better than it was before so I think it
should still go in.
min() and max() would first do an existence check. Unfortunately existence
checks can be slow in certain situations (like if the smartset is a list, and
quickly iterable in both ascending and descending directions, then doing an
existence check will start from the bottom, even if you want to check the
max()).
The fix is to not do the check, and just handle the error if it happens. In a
large repo, this speeds up:
hg log -r 'max(parents(. + .^) - (. + .^) & ::master)'
from 3.5s to 0.85s. That revset is contrived and just for testing. In our
real case we used 'bundle()' in place of '. + .^'
Interesting perf numbers for the revset benchmarks:
max(draft() and ::tip) => 0.027s to 0.0005s
max(author(lmoscovicz)) => 2.48s to 0.57s
min doesn't show any perf changes, but changing it as well will prevent a perf
regression in my next patch.
Result from revset benchmark
revset #0: draft() and ::tip
min max
0) 0.001971 0.001991
1) 0.001965 0.000428 21%
revset #1: ::tip and draft()
min max
0) 0.002017 0.001912
1) 0.001896 94% 0.000421 22%
revset #2: author(lmoscovicz)
min max
0) 1.049033 1.358913
1) 1.042508 0.319824 23%
revset #3: author(lmoscovicz) or author(mpm)
min max
0) 1.042512 1.367432
1) 1.019750 0.327750 23%
revset #4: author(mpm) or author(lmoscovicz)
min max
0) 1.050135 0.324924
1) 1.070698 0.319913
revset #5: roots((tip~100::) - (tip~100::tip))
min max
0) 0.000671 0.001018
1) 0.000605 90% 0.000946 92%
revset #6: roots((0::) - (0::tip))
min max
0) 0.149714 0.152369
1) 0.098677 65% 0.100374 65%
revset #7: (20000::) - (20000)
min max
0) 0.051019 0.042747
1) 0.035586 69% 0.016267 38%
This is another step toward having "default" destination more clear and unified.
Not all the logic is there because some bookmark related computation happened
elsewhere. It will be moved later.
The function is private because as for the other ones, cleanup is needed before
we can proceed.
Since ca895be75c36, condition function returns a cached value, so there's
little benefit to cache __contains__.
No measurable difference found in contrib/base-revsets.txt.
When using multiple revsets that get optimized into a list (like
hg log -r r1235 -r r1237 in hgsubversion), the revset list code was assuming the
strings were resolvable via repo[X]. hgsubversion and other extensions override
def stringset() to allow processing different revision identifiers (such as
r1235 or g<githash>), and there for the _list() implementation was circumventing
that resolution.
The fix is to just call stringset(). The default implementaiton does the same
thing that _list was already doing (namely repo[X]).
This has always been broken, but it was recently exposed by ad142c72c6db which
made "--rev X --rev Y" produce a combined revset "X | Y".
Before this patch, follow only supports full, exact filenames.
This patch makes follow argument to be treated like file
pattern same way like log treats their arguments.
It preserves current behaviour of follow() matching paths
relative to the repository root by default.
As the content of a smartset never changes, min and max will never change
either. This will save us time when this function is called multiple times.
This is relevant for issue4782 but does not fix it.
Changeset 79b4c33e868f uses smartset lazy sorting for the C version. We need to
apply the same to the pure version for consistency. This is fixing the tests
with --pure.
Baseset needs a list to operate, but will convert that list back to a set for
membership testing. It seems a bit silly to convert the set into a list to
convert it back afterward.
The main goal of this patch series is to reduce the use of PyXxx() function
that is likely to require ugly error handling and inc/decref. Plus, this is
faster than using PySet_Contains().
revset #0: 0::tip
0) 0.004168
1) 0.003678 88%
This patch ignores out-of-range roots as they are in the pure implementation.
Because reachable sets are calculated from heads, and out-of-range heads raise
IndexError, we can just take out-of-range roots as unreachable. Otherwise,
the test of "hg log -Gr '. + wdir()'" would fail.
"heads" argument is changed to a list. Should we have to rename the C function
as its signature is changed?
This patch is part of a series of patches to speed up the computation of
revset.reachableroots by introducing a C implementation. The main motivation is to
speed up smartlog on big repositories. At the end of the series, on our big
repositories the computation of reachableroots is 10-50x faster and smartlog on is
2x-5x faster.
Before this patch, reachableroots was computed in pure Python by default. This
patch makes the C implementation the default and provides a speedup for
reachableroots.
This patch is part of a series of patches to speed up the computation of
revset.revsbetween by introducing a C implementation. The main motivation is to
speed up smartlog on big repositories. At the end of the series, on our big
repositories the computation of revsbetween is 10-50x faster and smartlog on is
2x-5x faster.
This patch rename 'revsbetween' to 'reachableroots' and makes the computation of
the full path optional. This will allow graphlog to compute grandparents using
'reachableroots' and remove the need for a dedicated grandparent function.
This patch is part of a series of patches to speed up the computation of
revset.revsbetween by introducing a C implementation. The main motivation is to
speed up smartlog on big repositories. At the end of the series, on our big
repositories the computation of revsbetween is 10-50x faster and smartlog on is
2x-5x faster.
Later in this serie, we want to reuse the implementation of revsbetween in the
changelog module, therefore, we make it public.
An empty group expression "()" generates None in AST, so it should be tested
before destructuring a tuple.
"A | ()" is still evaluated to an error because I'm not sure whether "()"
represents an empty set or an empty expression (= a unit value). They are
identical in "or" operation, but they should be evaluated differently in
"and" operation.
expression empty set unit value
---------- --------- ----------
() {} A
A & () {} A
A | () A A
An empty group expression "()" generates None in AST, so the optimizer have
to test it before destructuring a tuple. The error message, "missing argument",
is somewhat obscure, but it should be better than crash.
This will allow us to define both a primary expression, ":", and a prefix
operator, ":y". The ambiguity will be resolved by the next patch.
Prefix actions in elements table are adjusted as follows:
original prefix primary prefix
----------------- -------- -----------------
("group", 1, ")") -> n/a ("group", 1, ")")
("negate", 19) -> n/a ("negate", 19)
("symbol",) -> "symbol" n/a
This is the simplest way to handle wdir() revision in revset. None didn't
work well because revset heavily depends on integer operations such as min(),
max(), sorted(), x:y, etc.
One downside is that we cannot do "wctx.rev() in set" because wctx.rev() is
still None. We could wrap the result set by wdirproxyset that translates None
to wdirrev, but it seems overengineered at this point.
result = getset(repo, subset, tree)
if 'wdir' in funcsused(tree):
result = wdirproxyset(result)
Test cases need the '(all() + wdir()) &' hack because we have yet to fix the
bootstrapping issue of null and wdir.
As already demonstrated, saving attribute lookup gains us some minor but
noticeable performance improvements.
revset #0: parents(all())
before) 0.024169
after ) 0.022756 94%
Keyword arguments will be convenient for functions that will take more than
one optional or boolean flags. For example,
file(pattern[, subrepos=false])
subrepo([[pattern], status])
Because I don't think all functions should accept key=value syntax, getkwargs()
does not support variadic functions such as 'ancestor(*changeset)'.
The core logic is placed in the parser module because keyword arguments will
be more useful in the templater, where functions take more options. Test cases
will be added by the next patch.
It will be used as an keyword argument.
Note that our "=" operator is left-associative. In general, the assignment
operator is right-associative, but we don't care because it isn't allowed to
chain "=" operations.
The crash was "TypeError: expected string or Unicode object, NoneType found"
down in revlog.parentrevs(). This fixes heads() too (which is where I found
it.)
Python 2.6 introduced the "except type as instance" syntax, replacing
the "except type, instance" syntax that came before. Python 3 dropped
support for the latter syntax. Since we no longer support Python 2.4 or
2.5, we have no need to continue supporting the "except type, instance".
This patch mass rewrites the exception syntax to be Python 2.6+ and
Python 3 compatible.
This patch was produced by running `2to3 -f except -w -n .`.
This can simplify the interface of parse() function. Our tokenizer tends to
have optional arguments other than the message to be parsed.
Before this patch, the "lookup" argument existed only for the revset, and the
templater had to pack [program, start, end] to be passed to its tokenizer.
The canonical way of doing 'roots(X)' is 'X - children(X)'. This is what the
implementation used to be. However, computing children is expensive because it
is unbounded. Any changesets in the repository may be a children of '0' so you
have to look at all changesets in the repository to compute children(0).
Moreover the current revsets implementation for children is not lazy, leading to
bad performance when fetching the first result.
There is a more restricted algorithm to compute roots:
roots(X) = [r for r in X if not parents(r) & X]
This achieve the same result while only looking for parent/children relation in
the X set itself, making the algorithm 'O(len(X))' membership operation.
Another advantages is that it turns the check into a simple filter, preserving
all laziness property of the underlying revsets.
The speed is very significant and some laziness is restored.
-) revset without 'roots(...)' to compare to base line
0) before this change
1) after this change
revset #0: roots((tip~100::) - (tip~100::tip))
plain min last
-) 0.001082 0.000993 0.000790
0) 0.001366 0.001385 0.001339
1) 0.001257 92% 0.001028 74% 0.000821 61%
revset #1: roots((0::) - (0::tip))
plain min last
-) 0.134551 0.144682 0.068453
0) 0.161822 0.171786 0.157683
1) 0.137583 85% 0.146204 85% 0.070012 44%
revset #2: roots(tip~100:)
plain min first last
-) 0.000219 0.000225 0.000231 0.000229
0) 0.000513 0.000529 0.000507 0.000539
1) 0.000463 90% 0.000269 50% 0.000267 52% 0.000463 85%
revset #3: roots(:42)
plain min first last
-) 0.000119 0.000146 0.000146 0.000146
0) 0.000231 0.000254 0.000253 0.000260
1) 0.000216 93% 0.000186 73% 0.000184 72% 0.000244 93%
revset #4: roots(not public())
plain min first
-) 0.000478 0.000502 0.000504
0) 0.000611 0.000639 0.000634
1) 0.000604 0.000560 87% 0.000558
revset #5: roots((0:tip)::)
plain min max first last
-) 0.057795 0.004905 0.058260 0.004908 0.038812
0) 0.132845 0.118931 0.130306 0.114280 0.127742
1) 0.111659 84% 0.005023 4% 0.111658 85% 0.005022 4% 0.092490 72%
revset #6: roots(0::tip)
plain min max first last
-) 0.032971 0.033947 0.033460 0.032350 0.033125
0) 0.083671 0.081953 0.084074 0.080364 0.086069
1) 0.074720 89% 0.035547 43% 0.077025 91% 0.033729 41% 0.083197
revset #7: 42:68 and roots(42:tip)
plain min max first last
-) 0.006827 0.000251 0.006830 0.000254 0.006771
0) 0.000337 0.000353 0.000366 0.000350 0.000366
1) 0.000318 94% 0.000297 84% 0.000353 0.000293 83% 0.000351
revset #8: roots(0:tip)
plain min max first last
-) 0.002119 0.000145 0.000147 0.000147 0.000147
0) 0.047441 0.040660 0.045662 0.040284 0.043435
1) 0.038057 80% 0.000187 0% 0.034919 76% 0.000186 0% 0.035097 80%
revset #0: roots(:42 + tip~42:)
plain min max first last sort
-) 0.000321 0.000317 0.000319 0.000308 0.000369 0.000343
0) 0.000772 0.000751 0.000811 0.000750 0.000802 0.000783
1) 0.000632 81% 0.000369 49% 0.000617 76% 0.000358 47% 0.000601 74% 0.000642 81%
I noticed when I mistyped 'matching', that it suggested '_matchfiles' as well.
Rather than simply exclude names that start with '_', this excludes anything
without a docstring. That way, if it isn't in the help text, it isn't
suggested, such as 'wdir()'.
If the computation of a set for each phase (done in C) is available,
we use it directly instead of applying a simple filter. This give a
massive speed-up in the vast majority of cases.
On my mercurial repo with about 15000 out of 40000 draft changesets:
revset: draft()
plain min first last
0) 0.011201 0.019950 0.009844 0.000074
1) 0.000284 2% 0.000312 1% 0.000314 3% 0.000315 x4.3
Bad performance for "last" come from the handling of the 15000 elements set
(memory allocation, filtering hidden changesets (99% of it) etc. compared to
applying the filter only on a handfuld of revisions (the first draft changesets
being close of tip).
This is not seen as an issue since:
* Timing is still pretty good and in line with all the other one,
* Current user of Vanilla Mercurial will not have 1/3 of their repo draft,
This bad effect disappears when phase's set is smaller. (about 200 secrets):
revset: secret()
plain min first last
0) 0.011181 0.022228 0.010851 0.000452
1) 0.000058 0% 0.000084 0% 0.000087 0% 0.000087 19%
Code for draft and secret are the same. We'll make it more complex to
take advantages of the set recomputed in C, so we first refactor the
code to only have one place to update (and make sure all behave
properly).
We do not refactor the 'public()' code because it does not have a natively
computed set.
Using 'repo[X]' is much slower because it creates a 'changectx' object and goes
though multiple layers of code to do so. It is also error prone if there is
tags, bookmarks, branch or other names that could map to a node hash and take
precedence (user are wicked).
This provides a significant performance boost on repository with a lot of
heads. Benchmark result for a repo with 1181 heads.
revset: head()
plain min last reverse
0) 0.014853 0.014371 0.014350 0.015161
1) 0.001402 9% 0.000975 6% 0.000874 6% 0.001415 9%
revset: head() - public()
plain min last reverse
0) 0.015121 0.014420 0.014560 0.015028
1) 0.001674 11% 0.001109 7% 0.000980 6% 0.001693 11%
revset: draft() and head()
plain min last reverse
0) 0.015976 0.014490 0.014214 0.015892
1) 0.002335 14% 0.001018 7% 0.000887 6% 0.002340 14%
The speed up is visible even when other more costly revset are in use
revset: head() and author("mpm")
plain min last reverse
0) 0.105419 0.090046 0.017169 0.108180
1) 0.090721 86% 0.077602 86% 0.003556 20% 0.093324 86%
The '_notpublic()' internal revset was "returning" a set. That was wrong. We now
return a 'baseset' as appropriate. This has no effect on performance in most case,
because we do the exact same operation than what the combination with a
'fullreposet' was doing. This as a small effect on some operation when combined
with other set, because we now apply the filtering in all cases. I think the
correctness is worth the impact on some corner cases. The optimizer should take
care of these corner cases anyway.
revset #0: not public()
plain min max first last reverse
0) 0.000465 0.000491 0.000495 0.000500 0.000494 0.000479
1) 0.000484 0.000503 0.000498 0.000505 0.000504 0.000491
revset #1: (tip~1000::) - public()
plain min max first last reverse
0) 0.002765 0.001742 0.002767 0.001730 0.002761 0.002782
1) 0.002847 0.001777 0.002776 0.001741 0.002764 0.002858
revset #2: not public() and branch("default")
plain min max first last reverse
0) 0.012104 0.011138 0.011189 0.011138 0.011166 0.011578
1) 0.011387 94% 0.011738 105% 0.014220 127% 0.011223 0.011184 0.012077
revset #3: (not public() - obsolete())
plain min max first last reverse
0) 0.000583 0.000556 0.000552 0.000555 0.000552 0.000610
1) 0.000613 105% 0.000559 0.000557 0.000573 0.000558 0.000613
revset #4: head() - public()
plain min max first last reverse
0) 0.010869 0.010800 0.011547 0.010843 0.010891 0.010891
1) 0.011031 0.011497 106% 0.011087 0.011100 0.011100 0.011085
If we are the very first rev access (or if the phase cache just got
invalidated) the phasesets will be None even if we support the native
computation. So we explicitly trigger a computation if needed.
This was not an issue before because requesting any phase information
would have triggered such computation.
As stated in the comment, using the smartset 'min' will give more opportunity to
be smart. It give a small but significant boost to the performance. Most of the
time is still spend doing the actual computation but at least we can scrap some
performance when it makes sense.
revset #0: roots(0:tip)
plain
0) 0.046600
1) 0.044109 94%
Python is slow at attributes lookup. No, really, I mean -slow-. prefetching
these three methods give use a measurable performance boost.
revset #0: 0::tip
plain
0) 0.037655
1) 0.034290 91%
Sets have non-defined order and this should break stuff, but as we are lucky
fullreposet is also broken so the result is "not too bad".
We should fix it anyway, but it is too much for my current plate.
Using smartset's min will be significantly faster when the input set can provided
an optimised answer. I do not have time to fix all of them but I'm marking the
spot.
I cannot fix all issues in revset because I've got other things to do,
but let's write down all the brokenness to help other people reading
and fixing.
As seen in issue4565 and issue4624, GUI wrappers and automated scripts are
likely to generate a long query that just has numeric revisions joined by 'or'.
One reason why is that they allows users to choose arbitrary revisions from
a list. Because this use case isn't handled well by smartset, let's optimize
it to a plain old list.
Benchmarks:
1) reduce nesting of chained 'or' operations
2) optimize to a list (this patch)
revset #0: 0 + 1 + 2 + ... + 1000
1) wall 0.483341 comb 0.480000 user 0.480000 sys 0.000000 (best of 20)
2) wall 0.025393 comb 0.020000 user 0.020000 sys 0.000000 (best of 107)
revset #1: sort(0 + 1 + 2 + ... + 1000)
1) wall 0.035240 comb 0.040000 user 0.040000 sys 0.000000 (best of 100)
2) wall 0.026432 comb 0.030000 user 0.030000 sys 0.000000 (best of 102)
revset #2: first(0 + 1 + 2 + ... + 1000)
1) wall 0.028949 comb 0.030000 user 0.030000 sys 0.000000 (best of 100)
2) wall 0.025503 comb 0.030000 user 0.030000 sys 0.000000 (best of 106)
This is the same workaround introduced at 3aa0e4733e6f. Without this patch,
"null or x" can't be optimized to _list(null x).
Test case will be added by the next patch.
This allows us to optimize chained 'or' operations to _list() expression.
Unlike _intlist() or _hexlist(), it's difficult to remove duplicates by the
caller of _list() because different symbols can point to the same revision.
If the caller knows all symbols are unique, that probably means revisions or
nodes are known, therefore, _intlist() or _hexlist() should be used instead.
So, it makes sense to check duplicates by _list() function.
'%ls' is no longer used in core, this won't cause performance regression.
This reduces the stack depth of chained 'or' operations:
- from O(n) to O(1) at the parsing, alias expansion and optimization phases
- from O(n) to O(log(n)) at the evaluation phase
simplifyinfixops() must be applied immediately after the parsing phase.
Otherwise, alias expansion would crash by "maximum recursion depth exceeded"
error.
Test cases use 'x:y|y:z' instead of 'x|y' because I'm planning to optimize
'x|y' in a different way.
Benchmarks:
0) a2acce8dcd95
1) this patch
revset #0: 0 + 1 + 2 + ... + 200
0) wall 0.026347 comb 0.030000 user 0.030000 sys 0.000000 (best of 101)
1) wall 0.023858 comb 0.030000 user 0.030000 sys 0.000000 (best of 112)
revset #1: 0 + 1 + 2 + ... + 1000
0) maximum recursion depth exceeded
1) wall 0.483341 comb 0.480000 user 0.480000 sys 0.000000 (best of 20)
revset #2: sort(0 + 1 + 2 + ... + 200)
0) wall 0.013404 comb 0.010000 user 0.010000 sys 0.000000 (best of 196)
1) wall 0.006814 comb 0.010000 user 0.010000 sys 0.000000 (best of 375)
revset #3: sort(0 + 1 + 2 + ... + 1000)
0) maximum recursion depth exceeded
1) wall 0.035240 comb 0.040000 user 0.040000 sys 0.000000 (best of 100)
This function will be used by revset.orset() and scmutil.revrange() to reduce
the stack depth from O(n) to O(log(n)).
We've bikeshed the interface of this function, but we couldn't come to an
agreement. So we decided to attempt to make it move forward.
marmoute:
- new factory function isn't necessary for balanced addsets
- addset.__init__ can just recurse, should handle "len(subsets) == 2+"
yuja:
- want to write all "len(subsets) == 0, 1, 2, 3+" cases in the same function
- no recursion in __init__ for cosmetic reason: can't return, can't call
__init__ directly
I've changed it to a private function so that nobody would be tempted to
utilize it.
Though the original code did nothing, it tried to optimize the calculation
order by weight. But we can't simply swap 'ta' and 'tb' because it would
change the order of revisions.
For future reference, this patch keeps the modified version of the original
code as comment.
This patch partially backs out a9a86cbbc5b2 and adds an alternative workaround
to functions that evaluate "null" and "wdir()". Because the new workaround is
incomplete, "first(null)" and "min(null)" don't work as expected. But they were
not usable until 3.4 and "null" isn't commonly used, we can postpone a complete
fix for 3.5.
The issue4682 was caused because "branch(default)" is evaluated to
"<filteredset <fullreposet>>", keeping fullreposet magic. The next patch will
fix crash on "branch(null)", but without this patch, it would make
"null in <branch(default)>" be True, which means "children(branch(default))"
would return all revisions but merge (p2 != null).
I believe the right fix is to stop propagating fullreposet magic on filter(),
but it wouldn't fit to stable release. Also, we should discuss how to handle
"null" and "wdir()" in revset before.
revset.parse() should be responsible for all parsing errors. Perhaps it wasn't
because 'revset.parse' was not a real function when the validation code was
added at ac01134d0a40.
The patch solves two issues:
1. id(unknown_full_hash) aborts, but id(unknown_short_hash) doesn't
2. id(40byte_tag_or_bookmark) returns tagged/bookmarked revision,
but id(non-40byte_tag_or_bookmark) doesn't
After the patch:
1. id(unknown_full_hash) doesn't abort
2. id(40byte_tag_or_bookmark) returns empty set
wdir() implementation is still incomplete and shouldn't be advertised to
users. This patch will be backed out when
- template values such as {rev} and {node} are settled
- major commands and revsets work without crashing
I came across a case where an internal command was using a revset that I didn't
immediately pass in and it was difficult to debug what was going wrong with the
revset. This prints out the revset and informs the user that the error is with
a rebset so it should be more obvious what and where the error is.
This will be useful to execute actions after the tree is parsed and
before the revset returns a match. Finding symbols in the parse tree
will later allow hashes of hidden revisions to work on the command
line without the --hidden flag.
If self is a smartset and other is a fullreposet, nothing should be necessary.
A small win for trivial query in mozilla-central repo:
revset #0: (0:100000)
0) wall 0.017211 comb 0.020000 user 0.020000 sys 0.000000 (best of 163)
1) wall 0.001324 comb 0.000000 user 0.000000 sys 0.000000 (best of 2160)
This returns the csets where matching subrepos have changed with respect to the
containing repo's first parent. The second parent shouldn't matter, because it
is either syncing up to the first parent (i.e. it hasn't changed from the
current branch's POV), or the merge changed it with respect to the first parent
(which already adds it to the set).
There's already a 'subrepo' fileset, but it is prefixed with 'set:', so there
should be no ambiguity (in code anyway). The only test I see for it is to
revert subrepos named by a glob pattern (in test-subrepo.t, line 58). Since it
doesn't return a tracked file, neither 'log "set:subrepo()"' nor
'files "set:subrepo()"' print anything. Therefore, it seems useful to have a
revset that will return something for log (and can be added to a revsetalias to
be chained with 'file' revsets.)
It might be nice to be able to filter for added, modified and removed
separately, but add/remove should be rare. It might also be nice to be able to
do a 'contains' check, in addition to this mutated check. Maybe it is possible
to get those with the existing 'adds', 'contains', 'modifies' and 'removes' by
teaching them to chase explicit paths into subrepos.
I'm not sure if this should be added to the 'modifies adds removes' line in
revset.optimize() (since it is doing an AMR check on .hgsubstate), or if it is
OK to put into 'safesymbols' (things like 'file' are on the list, and that takes
a regex, among other patterns).
This patvh speeds up the computation of the not public() changeset
and incidentally speed up the computation of divergents() changeset on our big
repo by 100x from 50% to 0.5% of the time spent in smartlog with evolve.
In this patch we optimize not public() to _notpublic() (new revset) and use
the work on phaseset (from the previous commit) to be able to compute
_notpublic() quickly.
We use a non-lazy approach making the assumption the number of notpublic
change will not be in the order of magnitude of the repo size. Adopting a
lazy approach gives a speedup of 5x (vs 100x) only due to the overhead of the
code for lazy generation.