Previously revset weight is hardcoded and cannot be modified. This patch
moves it to predicate so newly registered revsets could define their weight
to properly give static optimization some hint.
Details
- Reviewers
phillco krbullock singhsrb yuja - Group Reviewers
hg-reviewers - Commits
- rHGb0790bebfcf8: revset: move weight information to predicate
Diff Detail
- Repository
- rHG Mercurial
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
lgtm
mercurial/revset.py | ||
---|---|---|
256 | I take it this dictionary was moved there because of the import direction? |
mercurial/revset.py | ||
---|---|---|
256 | Yes. |
Perhaps we'll need to make these magic numbers less obfuscated. Before, weights
were defined in one location and functions were grouped, so it wasn't hard to guess
what sort of functions should belonged to e.g. the w=30 group. With this patch,
weights can be arbitrary chosen by extension authors, but could we choose the
right value?
How about trying to document it? ex.
'weight' is an estimated number for static optimization so things like "given 'x & y', calculate x first or y first?" could be decided. 'weight' has a default value 10. revset with O(changelog.d) complexity usually have weight 100, revset which also needs reading manifest diffs usually have weight 30, revset reading file contents usually have weight 100. For a similar time complexity, revset returning less revisions could have less weight.
I think either expanding the docs or using constants would be fine, but the default weight looks like it's 1, not 10.
I think it's hard to use constants since that would implicitly limit the possibilities. Time complexity choices are unbounded in theory. So I'll go the documentation approach.
(copied from the email thread, though I'm okay with the latest patch.)
On Fri, 8 Sep 2017 18:24:44 +0000, phillco (Phil Cohen) wrote:
phillco added a comment.
Constants could pobably help make it more self-documenting.
Perhaps. And maybe we can add a debug command that shows a list of revset
functions sorted by weight.
mercurial/registrar.py | ||
---|---|---|
175 | contains() does not read file contents, but manifest contents. |
contains() does not read file contents, but manifest contents.