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.
Before this change, doing ordered iteration over an 'addset' object composed of
operands without fastasc or fastdesc method could result in duplicated entries.
This was the result of applying '_iterordered' on an unordered set.
We fix it by ensuring we iterate over the set in a sorted order. Using the fast
iterator when it exists on any operand. We kill the '_iterator' method in the
process because it did not make a lot of sense independently.
Thanks goes to Yuya Nishihara for reporting the issue and analysing the cause.
The main purpose of wdir() is to annotate working-directory files.
Currently many commands and revsets cannot handle workingctx and may raise
exception. For example, -r ":wdir()" results in TypeError. This problem will
be addressed by future patches.
We could add "wdir" symbol instead, but it would conflict with the existing
tag, bookmark or branch. So I decided not to.
List of commands that will potentially support workingctx revision:
command default remarks
-------- ------- -----------------------------------------------------
annotate p1 useful
archive p1 might be useful
cat p1 might be useful on Windows (no cat)
diff p1:wdir (default)
export p1 might be useful if wctx can have draft commit message
files wdir (default)
grep tip:0 might be useful
identify wdir (default)
locate wdir (default)
log tip:0 might be useful with -p or -G option
parents wdir (default)
status wdir (default)
This patch includes minimal test of "hg status" that should be able to handle
the workingctx revision.
Previously we would instantiate the revbranchcache with a repo object, use it
briefly, then require it be passed in every time we wanted to fetch any
information. This seems unnecessary since it's obviously specific to that repo
(since it was constructed with it).
This patch stores the repo on the revbranchcache object, and removes the repo
parameter from the various functions on that class. This has the other nice
benefit of removing the double-revbranchcache-read that existed before (it was
read once for the branch revset, and once for the repo.revbranchcache).
Although Python supports `X = Y if COND else Z`, this was only
introduced in Python 2.5. Since we have to support Python 2.4, it was
a very common thing to write instead `X = COND and Y or Z`, which is a
bit obscure at a glance. It requires some intricate knowledge of
Python to understand how to parse these one-liners.
We change instead all of these one-liners to 4-liners. This was
executed with the following perlism:
find -name "*.py" -exec perl -pi -e 's,(\s*)([\.\w]+) = \(?(\S+)\s+and\s+(\S*)\)?\s+or\s+(\S*)$,$1if $3:\n$1 $2 = $4\n$1else:\n$1 $2 = $5,' {} \;
I tweaked the following cases from the automatic Perl output:
prev = (parents and parents[0]) or nullid
port = (use_ssl and 443 or 80)
cwd = (pats and repo.getcwd()) or ''
rename = fctx and webutil.renamelink(fctx) or []
ctx = fctx and fctx or ctx
self.base = (mapfile and os.path.dirname(mapfile)) or ''
I also added some newlines wherever they seemd appropriate for readability
There are probably a few ersatz ternary operators still in the code
somewhere, lurking away from the power of a simple regex.
As per fullreposet.__and__, it can omit the range check of rev. Therefore,
"null" revision is accepted automagically.
It seems this can fix many query results involving null symbol. Originally,
the simplest "(null)" query did fail if there were hidden revisions. Tests
are randomly chosen.
fullreposet mimics the behavior of localrepo, where "null" revision is not
listed but contained.
fcccbf073394 says we should avoid function calls in __contains__, so
super(fullreposet, self).__contains__(rev) is not an option.
Actually the super call doubled the benchmark result of trivial query:
revisions:
0) 6aa81b0c4658 (tip when I wrote this patch)
1) rev == node.nullrev or super(fullreposet, self).__contains__(rev)
revset #0: tip:0
0) wall 0.008441 comb 0.010000 user 0.010000 sys 0.000000 (best of 282)
1) wall 0.016152 comb 0.010000 user 0.010000 sys 0.000000 (best of 146)
I'm not sure if "all()" should filter out "null", but "all()" is stated as
'the same as "0:tip"' (except that it doesn't reorder the subset, I think.)
This patch is intended to avoid exposing a fullreposet to graphmod.dagwalker(),
which would result in strange drawing in future version:
|
o changeset: 0:f8035bb17114
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: add a
caused by:
parents = sorted(set([p.rev() for p in ctx.parents()
if p.rev() in revs]))
We cannot add "and p.rev() != nullrev" here because revs may actually include
"null" revision.
Transitioning to Mercurial versions with revision branch cache could be slow as
long as all operations were readonly (revset queries) and the cache would be
populated but not written back.
Instead, fall back to using the consistently slow path when readonly and the
cache doesn't exist yet. That avoids the overhead of populating the cache
without writing it back.
If not readonly, it will still populate all missing entries initially. That
avoids repeated writing of the cache file with small updates, and it also makes
sure a fully populated cache available for the readonly operations.
Before this patch, revset predicate "tag()" and "named('tags')" differ
from each other, because the former doesn't include "tip" but the
latter does.
For equivalence, "named('tags')" shouldn't include the revision
corresponded to "tip". But just removing "tip" from the "tags"
namespace causes breaking backward compatibility, even though "tip"
itself is planned to be eliminated, as mentioned below.
http://selenic.com/pipermail/mercurial-devel/2015-February/066157.html
To mask specific names ("tip" in this case) for "named()" predicate,
this patch introduces "deprecated" into "namespaces", and makes
"named()" predicate examine whether each names are masked by the
namespace, to which they belong.
"named()" will really work correctly after 3.3.1 (see a3c326a7f57a for
detail), and fixing this on STABLE before 3.3.1 can prevent initial
users of "named()" from expecting "named('tags')" to include "tip".
It is reason why this patch is posted for STABLE, even though problem
itself isn't so serious.
This may have to be flagged as "(BC)", if applied on DEFAULT.
Before this patch, revset predicate "named()" uses each nodes gotten
from target namespaces directly.
This causes problems below:
- combination of other predicates doesn't work correctly, because
they assume that revisions are listed up in number
- "hg log" doesn't show any revisions for "named()" result, because:
- "changeset_printer" stores formatted output for each revisions
into dict with revision number (= ctx.rev()) as a key of them
- "changeset_printer.flush(rev)" writes stored output for
the specified revision, but
- "commands.log" invokes it with the node, gotten from "named()"
- "hg debugrevspec" shows nodes (= may be binary) directly
Difference between revset predicate "tag()" and "named('tags')" in
tests is fixed in subsequent patch.
Before this patch, "bookmark()", "named()" and "tag()" predicates
raise "Abort", when the specified pattern doesn't match against
existing ones.
This prevents "present()" predicate from continuing the query, because
it only catches "RepoLookupError".
This patch raises "RepoLookupError" instead of "Abort", to make
"present()" predicate continue the query, even if "bookmark()",
"named()" or "tag()" in the sub-query of it are aborted.
This patch doesn't contain raising "RepoLookupError" for "re:" pattern
in "tag()", because "tag()" treats it differently from others. Actions
of each predicates at failure of pattern matching can be summarized as
below:
predicate "literal:" "re:"
---------- ----------- ------------
bookmark abort abort
named abort abort
tag abort continue (*1)
branch abort continue (*2)
---------- ----------- ------------
"tag()" may have to abort in the (*1) case for similarity, but this
change may break backward compatibility of existing revset queries. It
seems to have to be changed on "default" branch (with "BC" ?).
On the other hand, (*2) seems to be reasonable, even though it breaks
similarity, because "branch()" in this case doesn't check exact
existence of branches, but does pick up revisions of which branch
matches against the pattern.
This patch also adds tests for "branch()" to clarify behavior around
"present()" of similar predicates, even though this patch doesn't
change "branch()".
This can simplify the conversion from numeric revision to string. Without it,
we have to handle -1 specially because repo['-1'] != repo[-1].
The -1 revision is not officially documented, but this change makes sense
assuming that "rev(%d)" exists for scripting or third-party tools.
When running "hg log 'set:added()'", we create two matchers: one used
for producing the revset and one used for finding files to match. In
185b6b930e8c (graphlog: evaluate FILE/-I/-X filesets on the working
dir, 2012-02-26), we started passing a revision argument along from
what's currently in cmdutil._makelogrevset() to
revset._matchfiles(). When the revision was an empty string, it
referred to the working copy. This was subtly done with "repo[rev or
None]". Then, in 5ff5c5c9e69f (revset: avoid recalculating filesets,
2014-10-22), that conversion from empty string to None was lost. Note
that repo[''] is equivalent to repo['.'], not repo[None].
The consequence of this, to the user, is that when running "hg log
'set:added()'", the file matcher matches files added in the working
copy, while the revset matcher matches revisions that touch files
added in the parent of the working copy. As a result, only revisions
that touch any files added in the parent of the working copy will be
considered, but they will only be included if they also touch files
added in the working copy.
Fix the bug by converting '' to None again, but make it a little more
explicit this time (plus, we now have tests for it).
This change is intended to avoid exposing the implementation detail to
callers. I'm going to extend fullreposet to support "null" revision, so
these mfunc calls will have to use fullreposet() instead of spanset().
Before this patch, collisions between alias argument names in the
declaration are ignored, and this silently causes unexpected alias
evaluation.
This patch checks for such collisions, and aborts (or shows a warning) when
collisions are detected.
This patch doesn't add a test to "test-revset.t", because a doctest is
enough to test the collisions detection itself.
Before this patch, alias declaration is parsed by string base
operations: matching against "^([^(]+)\(([^)]+)\)$" and splitting by
",".
This overlooks many syntax errors like below (see the previous patch
introducing "_parsealiasdecl" for detail):
- un-closed parenthesis causes being treated as "alias symbol"
- symbol/function name aren't examined whether they are valid or not
- invalid argument list causes unexpected argument names
To parse alias declaration strictly, this patch replaces parsing
implementation by "_parsealiasdecl".
This patch tests only one typical declaration error case, because
error detection itself is already tested in the doctest of
"_parsealiasdecl".
This also removes class property "args" and "error", because these are
certainly initialized in "revsetalias.__init__".
This patch introduces "_parsealiasdecl" to parse alias declarations
strictly. For example, "_parsealiasdecl" can detect problems below,
which current implementation can't.
- un-closed parenthesis causes being treated as "alias symbol"
because all of declarations not in "func(....)" style are
recognized as "alias symbol".
for example, "foo($1, $2" is treated as the alias symbol.
- alias symbol/function names aren't examined whether they are valid
as symbol or not
for example, "foo bar" can be treated as the alias symbol, but of
course such invalid symbol can't be referred in revset.
- just splitting argument list by "," causes overlooking syntax
problems in the declaration
for example, all of invalid declarations below are overlooked:
- foo("bar") => taking one argument named as '"bar"'
- foo("unclosed) => taking one argument named as '"unclosed'
- foo(bar::baz) => taking one argument named as 'bar::baz'
- foo(bar($1)) => taking one argument named as 'bar($1)'
To decrease complication of patch, current implementation for alias
declarations is replaced by "_parsealiasdecl" in the subsequent
patch. This patch just introduces it.
This patch defines "_parsealiasdecl" not as a method of "revsetalias"
class but as a one of "revset" module, because of ease of testing by
doctest.
This patch factors some helper functions for "tree" out, because:
- direct accessing like "if tree[0] == 'func' and len(tree) > 1"
decreases readability
- subsequent patch (and also existing code paths, in the future) can
use them for readability
This patch also factors "_tokenizealias" out, because it can be used
also for parsing alias definitions strictly.
Before this patch, any errors in the declaration of revset alias
aren't detected at all, and there is no information about error source
in the error message.
As a part of preparation for parsing alias declarations and
definitions more strictly, this patch stores full detail into
"revsetalias.error" for error source distinction.
This makes raising "Abort" and warning potential errors just use
"revsetalias.error" without any message composing.
This patch defines the composing function not in "ParseError" class but
in "revset" module, because:
- "_()" shouldn't be used in "ParseError", to avoid adding "from
i18n import _" i18n" to "error" module
- generalizing message composition of"ParseError" for all code paths
other than revset isn't the purpose of this patch
we should also take care of showing "unexpected leading
whitespace" for some code paths, to generalize widely.
Before this patch, "tokenize" doesn't recognize the symbol starting
with "$" as a valid one.
This prevents revset alias declarations and definitions from being
parsed with "tokenize", because "$" may be used as the initial letter
of alias arguments.
BTW, the alias argument name doesn't require leading "$" itself, in
fact. But we have to assume that users may use "$" as the initial
letter of argument names in their aliases, because examples in "hg
help revsets" uses such names for a long time.
To make "tokenize" extensible to parse alias declarations and
definitions, this patch introduces optional arguments "syminitletters"
and "symletters". Giving these sets can change the policy of "valid
symbol" in tokenization easily.
This patch keeps original examination of letter validity for
reviewability, even though there is redundant interchanging between
"chr"/"ord" at initialization of "_syminitletters" and "_symletters".
At most 256 times examination (per initialization) is cheaper enough
than revset evaluation itself.
This patch is a part of preparation for parsing alias declarations and
definitions more strictly.
Because spanset.isascending() ignored the ascending flag, the result of
"fullreposet() & x" was always sorted in ascending order.
The test case is carefully chosen to call fullreposet.__and__.
Branch name filtering in revsets was expensive. For every rev it created a
changectx and called .branch() which retrieved the branch name from the
changelog.
Instead, use the revbranchcache.
The revbranchcache is used read-only. The revset implementation with generators
and callbacks makes it hard to figure out when we are done using/updating the
cache and could write it back. It would also be 'tricky' to lock the repo for
writing from within a revset execution. Finally, the branchmap update will
usually make sure that the cache is updated before any revset can be run.
The revbranchcache is used without any locking but is short-lived and used in a
tight loop where we can assume that the changelog doesn't change ... or where
it not is relevant to us if it does.
perfrevset 'branch(mobile)' on mozilla-central.
Before:
! wall 10.989637 comb 10.970000 user 10.940000 sys 0.030000 (best of 3)
After, no cache:
! wall 7.368656 comb 7.370000 user 7.360000 sys 0.010000 (best of 3)
After, with cache:
! wall 0.528098 comb 0.530000 user 0.530000 sys 0.000000 (best of 18)
The performance improvement even without cache come from being based on
branchinfo on the changelog instead of using ctx.branch().
Some tests are added to verify that the revbranchcache works and keep an eye on
when the cache files actually are updated.
It was introduced at deb42ca4dd93, where spanset.__contains__() did not exist.
Nowadays, we have to pay huge penalty for len(subset).
The following example showed that OR operation could be O(n * m^2)
(n: len(repo), m: number of OR operators, m >= 2) probably because of
filteredset.__len__.
revset #0: 0|1|2|3|4|5|6|7|8|9
0) wall 8.092713 comb 8.090000 user 8.090000 sys 0.000000 (best of 3)
1) wall 0.445354 comb 0.450000 user 0.430000 sys 0.020000 (best of 22)
2) wall 0.000389 comb 0.000000 user 0.000000 sys 0.000000 (best of 7347)
(0: 3.2.4, 1: 3.1.2, 2: this patch)
With this patch, we can make it much easier to specify 'only(A,B)' ->
A%B. Similarly, 'only(A)' -> A%.
On Windows, '%' is a semi-reserved symbol in the following way: using non-bash
shells (e.g. cmd.exe but NOT PowerShell, ConEmu, and cmder), %var% is only
expanded when 'var' exists and is surrounded by '%'.
That only leaves batch scripts which could prove to be problematic. I posit
that this isn't a big issue because any developer of batch scripts already
knows that to use '%' one needs to escape it by using a double '%%'.
Alternatives to '%' could be '=' but that might be limiting our future if we
ever decide to use temporary assignments in a revset.
fullreposet.__contains__() will be rewritten in order to support "null"
revision, and "rev()" won't be possible to rely on it.
This backs out 23ac42e12ce5, but there is no performance regression now.
revisions:
0) bd19f94d30e9 "l not in fullreposet(repo)"
1) this patch "l not in repo.changelog"
revset #0: rev(210000)
0) wall 0.000056 comb 0.000000 user 0.000000 sys 0.000000 (best of 48036)
1) wall 0.000049 comb 0.000000 user 0.000000 sys 0.000000 (best of 54969)
Before this patch, referring alias arguments is parsed by string base
operation "str.replace".
This causes problems below (see the previous patch introducing
"_parsealiasdefn" for detail)
- the shorter name argument breaks referring the longer name
- argument names in the quoted string are broken
This patch replaces parsing alias definition by "_parsealiasdefn" to
parse strictly.
This patch introduces "_parsealiasdefn" to parse alias definitions
strictly. For example, it can avoid problems below, which current
implementation can't.
- the shorter name argument breaks referring the longer name one in
the definition, if the former is completely prefix of the latter
for example, the alias definition "foo($1, $10) = $1 or $10" is
parsed as "_aliasarg('$1') or _aliasarg('$1')0" and causes parse
error, because tail "0" of "_aliasarg('$1')0" is invalid.
- argument names in the quoted string are broken
for example, the definition "foo($1) = $1 or desc('$1')" is parsed
as "_aliasarg('$1') or desc('_aliasarg(\'$1\')')" and causes
unexpected description matching against not '$1' but '_aliasarg(\'$1\')'.
To decrease complication of patch, current implementation for alias
definitions is replaced by "_parsealiasdefn" in the subsequent
patch. This patch just introduces it.
This patch defines "_parsealiasdefn" not as a method of "revsetalias"
class but as a one of "revset" module, because of ease of testing by
doctest.
Before this patch, there is no way to concatenate strings at runtime.
For example, to search for the issue ID "1234" in descriptions against
all of "issue 1234", "issue:1234", issue1234" and "bug(1234)"
patterns, the revset below should be written fully from scratch for
each issue ID.
grep(r"\bissue[ :]?1234\b|\bbug\(1234\)")
This patch introduces new infix operator "##" to concatenate
strings/symbols at runtime. Operator symbol "##" comes from the same
one of C pre-processor. This concatenation allows parametrizing a part
of strings in revset queries.
In the case of example above, the definition of the revset alias using
operator "##" below can search issue ID "1234" in complicated patterns
by "issue(1234)" simply:
issue($1) = grep(r"\bissue[ :]?" ## $1 ## r"\b|\bbug\(" ## $1 ## r"\)")
"##" operator does:
- concatenate not only strings but also symbols into the string
Exact distinction between strings and symbols seems not to be
convenience, because it is tiresome for users (and
"revset.getstring" treats both similarly)
For example of revset alias "issue()", "issue(1234)" is easier
than "issue('1234')".
- have higher priority than any other prefix, infix and postfix
operators (like as "##" of C pre-processor)
This patch (re-)assigns the priority 20 to "##", and 21 to "(",
because priority 19 is already assigned to "-" as prefix "negate".
The fix for linkrev pointing to hidden revision was crashing when the file was
missing from head's manifest. We now properly handle this case.
(yes I feel silly)