D657: revset: move weight information to predicate

2017-09-20 Thread quark (Jun Wu) 
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGb0790bebfcf8: revset: move weight information to predicate 
(authored by quark, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D657?vs=1896=1919

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

AFFECTED FILES
  mercurial/registrar.py
  mercurial/revset.py
  mercurial/revsetlang.py

CHANGE DETAILS

diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -49,6 +49,8 @@
 
 keywords = {'and', 'or', 'not'}
 
+symbols = {}
+
 _quoteletters = {'"', "'"}
 _simpleopletters = set(pycompat.iterbytestr("()[]#:=,-|&+!~^%"))
 
@@ -441,21 +443,7 @@
 elif op == 'func':
 f = getsymbol(x[1])
 wa, ta = _optimize(x[2])
-if f in ('author', 'branch', 'closed', 'date', 'desc', 'file', 'grep',
- 'keyword', 'outgoing', 'user', 'destination'):
-w = 10 # slow
-elif f in ('modifies', 'adds', 'removes'):
-w = 30 # slower
-elif f == "contains":
-w = 100 # very slow
-elif f == "ancestor":
-w = 0.5
-elif f in ('reverse', 'limit', 'first', 'wdir', '_intlist'):
-w = 0
-elif f == "sort":
-w = 10 # assume most sorts look at changelog
-else:
-w = 1
+w = getattr(symbols.get(f), '_weight', 1)
 return w + wa, (op, x[1], ta)
 raise ValueError('invalid operator %r' % op)
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -253,7 +253,7 @@
 #   repo - current repository instance
 #   subset - of revisions to be examined
 #   x - argument in tree form
-symbols = {}
+symbols = revsetlang.symbols
 
 # symbols which can't be used for a DoS attack for any given input
 # (e.g. those which accept regexes as plain strings shouldn't be included)
@@ -276,7 +276,7 @@
 sourceset = getset(repo, fullreposet(repo), x)
 return subset & baseset([destutil.destmerge(repo, sourceset=sourceset)])
 
-@predicate('adds(pattern)', safe=True)
+@predicate('adds(pattern)', safe=True, weight=30)
 def adds(repo, subset, x):
 """Changesets that add a file matching pattern.
 
@@ -288,7 +288,7 @@
 pat = getstring(x, _("adds requires a pattern"))
 return checkstatus(repo, subset, pat, 1)
 
-@predicate('ancestor(*changeset)', safe=True)
+@predicate('ancestor(*changeset)', safe=True, weight=0.5)
 def ancestor(repo, subset, x):
 """A greatest common ancestor of the changesets.
 
@@ -394,7 +394,7 @@
 ps.add(r)
 return subset & ps
 
-@predicate('author(string)', safe=True)
+@predicate('author(string)', safe=True, weight=10)
 def author(repo, subset, x):
 """Alias for ``user(string)``.
 """
@@ -462,7 +462,7 @@
 bms -= {node.nullrev}
 return subset & bms
 
-@predicate('branch(string or set)', safe=True)
+@predicate('branch(string or set)', safe=True, weight=10)
 def branch(repo, subset, x):
 """
 All changesets belonging to the given branch or the branches of the given
@@ -595,16 +595,16 @@
 cs = _children(repo, subset, s)
 return subset & cs
 
-@predicate('closed()', safe=True)
+@predicate('closed()', safe=True, weight=10)
 def closed(repo, subset, x):
 """Changeset is closed.
 """
 # i18n: "closed" is a keyword
 getargs(x, 0, 0, _("closed takes no arguments"))
 return subset.filter(lambda r: repo[r].closesbranch(),
  condrepr='')
 
-@predicate('contains(pattern)')
+@predicate('contains(pattern)', weight=100)
 def contains(repo, subset, x):
 """The revision's manifest contains a file matching pattern (but might not
 modify it). See :hg:`help patterns` for information about file patterns.
@@ -654,7 +654,7 @@
 return subset.filter(lambda r: _matchvalue(r),
  condrepr=('', rev))
 
-@predicate('date(interval)', safe=True)
+@predicate('date(interval)', safe=True, weight=10)
 def date(repo, subset, x):
 """Changesets within the interval, see :hg:`help dates`.
 """
@@ -664,7 +664,7 @@
 return subset.filter(lambda x: dm(repo[x].date()[0]),
  condrepr=('', ds))
 
-@predicate('desc(string)', safe=True)
+@predicate('desc(string)', safe=True, weight=10)
 def desc(repo, subset, x):
 """Search commit message for string. The match is case-insensitive.
 
@@ -722,7 +722,7 @@
 # Like ``descendants(set)`` but follows only the first parents.
 return _descendants(repo, subset, x, followfirst=True)
 
-@predicate('destination([set])', safe=True)
+@predicate('destination([set])', safe=True, weight=10)
 def destination(repo, subset, x):
 """Changesets that were created by a graft, transplant or rebase operation,
 with the given revisions specified as the source.  Omitting the optional 
set
@@ -891,7 +891,7 @@
 
 return subset &

D657: revset: move weight information to predicate

2017-09-20 Thread yuja (Yuya Nishihara) 
yuja accepted this revision.
yuja added a comment.


  Queued, thanks.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco, krbullock, singhsrb, yuja
Cc: krbullock, yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-19 Thread quark (Jun Wu) 
quark updated this revision to Diff 1896.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D657?vs=1895=1896

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

AFFECTED FILES
  mercurial/registrar.py
  mercurial/revset.py
  mercurial/revsetlang.py

CHANGE DETAILS

diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -49,6 +49,8 @@
 
 keywords = {'and', 'or', 'not'}
 
+symbols = {}
+
 _quoteletters = {'"', "'"}
 _simpleopletters = set(pycompat.iterbytestr("()[]#:=,-|&+!~^%"))
 
@@ -441,21 +443,7 @@
 elif op == 'func':
 f = getsymbol(x[1])
 wa, ta = _optimize(x[2])
-if f in ('author', 'branch', 'closed', 'date', 'desc', 'file', 'grep',
- 'keyword', 'outgoing', 'user', 'destination'):
-w = 10 # slow
-elif f in ('modifies', 'adds', 'removes'):
-w = 30 # slower
-elif f == "contains":
-w = 100 # very slow
-elif f == "ancestor":
-w = 0.5
-elif f in ('reverse', 'limit', 'first', 'wdir', '_intlist'):
-w = 0
-elif f == "sort":
-w = 10 # assume most sorts look at changelog
-else:
-w = 1
+w = getattr(symbols.get(f), '_weight', 1)
 return w + wa, (op, x[1], ta)
 raise ValueError('invalid operator %r' % op)
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -253,7 +253,7 @@
 #   repo - current repository instance
 #   subset - of revisions to be examined
 #   x - argument in tree form
-symbols = {}
+symbols = revsetlang.symbols
 
 # symbols which can't be used for a DoS attack for any given input
 # (e.g. those which accept regexes as plain strings shouldn't be included)
@@ -276,7 +276,7 @@
 sourceset = getset(repo, fullreposet(repo), x)
 return subset & baseset([destutil.destmerge(repo, sourceset=sourceset)])
 
-@predicate('adds(pattern)', safe=True)
+@predicate('adds(pattern)', safe=True, weight=30)
 def adds(repo, subset, x):
 """Changesets that add a file matching pattern.
 
@@ -288,7 +288,7 @@
 pat = getstring(x, _("adds requires a pattern"))
 return checkstatus(repo, subset, pat, 1)
 
-@predicate('ancestor(*changeset)', safe=True)
+@predicate('ancestor(*changeset)', safe=True, weight=0.5)
 def ancestor(repo, subset, x):
 """A greatest common ancestor of the changesets.
 
@@ -394,7 +394,7 @@
 ps.add(r)
 return subset & ps
 
-@predicate('author(string)', safe=True)
+@predicate('author(string)', safe=True, weight=10)
 def author(repo, subset, x):
 """Alias for ``user(string)``.
 """
@@ -462,7 +462,7 @@
 bms -= {node.nullrev}
 return subset & bms
 
-@predicate('branch(string or set)', safe=True)
+@predicate('branch(string or set)', safe=True, weight=10)
 def branch(repo, subset, x):
 """
 All changesets belonging to the given branch or the branches of the given
@@ -595,16 +595,16 @@
 cs = _children(repo, subset, s)
 return subset & cs
 
-@predicate('closed()', safe=True)
+@predicate('closed()', safe=True, weight=10)
 def closed(repo, subset, x):
 """Changeset is closed.
 """
 # i18n: "closed" is a keyword
 getargs(x, 0, 0, _("closed takes no arguments"))
 return subset.filter(lambda r: repo[r].closesbranch(),
  condrepr='')
 
-@predicate('contains(pattern)')
+@predicate('contains(pattern)', weight=100)
 def contains(repo, subset, x):
 """The revision's manifest contains a file matching pattern (but might not
 modify it). See :hg:`help patterns` for information about file patterns.
@@ -654,7 +654,7 @@
 return subset.filter(lambda r: _matchvalue(r),
  condrepr=('', rev))
 
-@predicate('date(interval)', safe=True)
+@predicate('date(interval)', safe=True, weight=10)
 def date(repo, subset, x):
 """Changesets within the interval, see :hg:`help dates`.
 """
@@ -664,7 +664,7 @@
 return subset.filter(lambda x: dm(repo[x].date()[0]),
  condrepr=('', ds))
 
-@predicate('desc(string)', safe=True)
+@predicate('desc(string)', safe=True, weight=10)
 def desc(repo, subset, x):
 """Search commit message for string. The match is case-insensitive.
 
@@ -722,7 +722,7 @@
 # Like ``descendants(set)`` but follows only the first parents.
 return _descendants(repo, subset, x, followfirst=True)
 
-@predicate('destination([set])', safe=True)
+@predicate('destination([set])', safe=True, weight=10)
 def destination(repo, subset, x):
 """Changesets that were created by a graft, transplant or rebase operation,
 with the given revisions specified as the source.  Omitting the optional 
set
@@ -891,7 +891,7 @@
 
 return subset & s
 
-@predicate('first(set, [n])', safe=True, takeorder=True)
+@predicate('first(set, [n])', safe=True, takeorder=True, weight=0)
 def first(repo,

D657: revset: move weight information to predicate

2017-09-19 Thread yuja (Yuya Nishihara) 
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  (copied from the email thread, though I'm okay with the latest patch.)
  
  On Fri, 8 Sep 2017 18:24:44 +, 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.

INLINE COMMENTS

> registrar.py:175
> +diffs usually have weight 30 (ex. adds); revset reading file contents
> +usually have weight 100 (ex. contains). Note: those values are not
> +absolute, if the revset has a same big-O time complexity as 'contains',

contains() does not read file contents, but manifest contents.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco, krbullock, singhsrb, yuja
Cc: krbullock, yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-18 Thread quark (Jun Wu) 
quark updated this revision to Diff 1883.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D657?vs=1882=1883

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

AFFECTED FILES
  mercurial/registrar.py
  mercurial/revset.py
  mercurial/revsetlang.py

CHANGE DETAILS

diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -49,6 +49,8 @@
 
 keywords = {'and', 'or', 'not'}
 
+symbols = {}
+
 _quoteletters = {'"', "'"}
 _simpleopletters = set(pycompat.iterbytestr("()[]#:=,-|&+!~^%"))
 
@@ -441,21 +443,7 @@
 elif op == 'func':
 f = getsymbol(x[1])
 wa, ta = _optimize(x[2])
-if f in ('author', 'branch', 'closed', 'date', 'desc', 'file', 'grep',
- 'keyword', 'outgoing', 'user', 'destination'):
-w = 10 # slow
-elif f in ('modifies', 'adds', 'removes'):
-w = 30 # slower
-elif f == "contains":
-w = 100 # very slow
-elif f == "ancestor":
-w = 0.5
-elif f in ('reverse', 'limit', 'first', 'wdir', '_intlist'):
-w = 0
-elif f == "sort":
-w = 10 # assume most sorts look at changelog
-else:
-w = 1
+w = getattr(symbols.get(f), '_weight', 1)
 return w + wa, (op, x[1], ta)
 raise ValueError('invalid operator %r' % op)
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -253,7 +253,7 @@
 #   repo - current repository instance
 #   subset - of revisions to be examined
 #   x - argument in tree form
-symbols = {}
+symbols = revsetlang.symbols
 
 # symbols which can't be used for a DoS attack for any given input
 # (e.g. those which accept regexes as plain strings shouldn't be included)
@@ -276,7 +276,7 @@
 sourceset = getset(repo, fullreposet(repo), x)
 return subset & baseset([destutil.destmerge(repo, sourceset=sourceset)])
 
-@predicate('adds(pattern)', safe=True)
+@predicate('adds(pattern)', safe=True, weight=30)
 def adds(repo, subset, x):
 """Changesets that add a file matching pattern.
 
@@ -288,7 +288,7 @@
 pat = getstring(x, _("adds requires a pattern"))
 return checkstatus(repo, subset, pat, 1)
 
-@predicate('ancestor(*changeset)', safe=True)
+@predicate('ancestor(*changeset)', safe=True, weight=0.5)
 def ancestor(repo, subset, x):
 """A greatest common ancestor of the changesets.
 
@@ -394,7 +394,7 @@
 ps.add(r)
 return subset & ps
 
-@predicate('author(string)', safe=True)
+@predicate('author(string)', safe=True, weight=10)
 def author(repo, subset, x):
 """Alias for ``user(string)``.
 """
@@ -462,7 +462,7 @@
 bms -= {node.nullrev}
 return subset & bms
 
-@predicate('branch(string or set)', safe=True)
+@predicate('branch(string or set)', safe=True, weight=10)
 def branch(repo, subset, x):
 """
 All changesets belonging to the given branch or the branches of the given
@@ -595,16 +595,16 @@
 cs = _children(repo, subset, s)
 return subset & cs
 
-@predicate('closed()', safe=True)
+@predicate('closed()', safe=True, weight=10)
 def closed(repo, subset, x):
 """Changeset is closed.
 """
 # i18n: "closed" is a keyword
 getargs(x, 0, 0, _("closed takes no arguments"))
 return subset.filter(lambda r: repo[r].closesbranch(),
  condrepr='')
 
-@predicate('contains(pattern)')
+@predicate('contains(pattern)', weight=100)
 def contains(repo, subset, x):
 """The revision's manifest contains a file matching pattern (but might not
 modify it). See :hg:`help patterns` for information about file patterns.
@@ -654,7 +654,7 @@
 return subset.filter(lambda r: _matchvalue(r),
  condrepr=('', rev))
 
-@predicate('date(interval)', safe=True)
+@predicate('date(interval)', safe=True, weight=10)
 def date(repo, subset, x):
 """Changesets within the interval, see :hg:`help dates`.
 """
@@ -664,7 +664,7 @@
 return subset.filter(lambda x: dm(repo[x].date()[0]),
  condrepr=('', ds))
 
-@predicate('desc(string)', safe=True)
+@predicate('desc(string)', safe=True, weight=10)
 def desc(repo, subset, x):
 """Search commit message for string. The match is case-insensitive.
 
@@ -722,7 +722,7 @@
 # Like ``descendants(set)`` but follows only the first parents.
 return _descendants(repo, subset, x, followfirst=True)
 
-@predicate('destination([set])', safe=True)
+@predicate('destination([set])', safe=True, weight=10)
 def destination(repo, subset, x):
 """Changesets that were created by a graft, transplant or rebase operation,
 with the given revisions specified as the source.  Omitting the optional 
set
@@ -891,7 +891,7 @@
 
 return subset & s
 
-@predicate('first(set, [n])', safe=True, takeorder=True)
+@predicate('first(set, [n])', safe=True, takeorder=True, weight=0)
 def first(repo,

D657: revset: move weight information to predicate

2017-09-18 Thread quark (Jun Wu) 
quark updated this revision to Diff 1882.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D657?vs=1666=1882

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

AFFECTED FILES
  mercurial/registrar.py
  mercurial/revset.py
  mercurial/revsetlang.py

CHANGE DETAILS

diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -49,6 +49,8 @@
 
 keywords = {'and', 'or', 'not'}
 
+symbols = {}
+
 _quoteletters = {'"', "'"}
 _simpleopletters = set(pycompat.iterbytestr("()[]#:=,-|&+!~^%"))
 
@@ -441,21 +443,7 @@
 elif op == 'func':
 f = getsymbol(x[1])
 wa, ta = _optimize(x[2])
-if f in ('author', 'branch', 'closed', 'date', 'desc', 'file', 'grep',
- 'keyword', 'outgoing', 'user', 'destination'):
-w = 10 # slow
-elif f in ('modifies', 'adds', 'removes'):
-w = 30 # slower
-elif f == "contains":
-w = 100 # very slow
-elif f == "ancestor":
-w = 0.5
-elif f in ('reverse', 'limit', 'first', 'wdir', '_intlist'):
-w = 0
-elif f == "sort":
-w = 10 # assume most sorts look at changelog
-else:
-w = 1
+w = getattr(symbols.get(f), '_weight', 1)
 return w + wa, (op, x[1], ta)
 raise ValueError('invalid operator %r' % op)
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -253,7 +253,7 @@
 #   repo - current repository instance
 #   subset - of revisions to be examined
 #   x - argument in tree form
-symbols = {}
+symbols = revsetlang.symbols
 
 # symbols which can't be used for a DoS attack for any given input
 # (e.g. those which accept regexes as plain strings shouldn't be included)
@@ -276,7 +276,7 @@
 sourceset = getset(repo, fullreposet(repo), x)
 return subset & baseset([destutil.destmerge(repo, sourceset=sourceset)])
 
-@predicate('adds(pattern)', safe=True)
+@predicate('adds(pattern)', safe=True, weight=30)
 def adds(repo, subset, x):
 """Changesets that add a file matching pattern.
 
@@ -288,7 +288,7 @@
 pat = getstring(x, _("adds requires a pattern"))
 return checkstatus(repo, subset, pat, 1)
 
-@predicate('ancestor(*changeset)', safe=True)
+@predicate('ancestor(*changeset)', safe=True, weight=0.5)
 def ancestor(repo, subset, x):
 """A greatest common ancestor of the changesets.
 
@@ -394,7 +394,7 @@
 ps.add(r)
 return subset & ps
 
-@predicate('author(string)', safe=True)
+@predicate('author(string)', safe=True, weight=10)
 def author(repo, subset, x):
 """Alias for ``user(string)``.
 """
@@ -462,7 +462,7 @@
 bms -= {node.nullrev}
 return subset & bms
 
-@predicate('branch(string or set)', safe=True)
+@predicate('branch(string or set)', safe=True, weight=10)
 def branch(repo, subset, x):
 """
 All changesets belonging to the given branch or the branches of the given
@@ -595,16 +595,16 @@
 cs = _children(repo, subset, s)
 return subset & cs
 
-@predicate('closed()', safe=True)
+@predicate('closed()', safe=True, weight=10)
 def closed(repo, subset, x):
 """Changeset is closed.
 """
 # i18n: "closed" is a keyword
 getargs(x, 0, 0, _("closed takes no arguments"))
 return subset.filter(lambda r: repo[r].closesbranch(),
  condrepr='')
 
-@predicate('contains(pattern)')
+@predicate('contains(pattern)', weight=100)
 def contains(repo, subset, x):
 """The revision's manifest contains a file matching pattern (but might not
 modify it). See :hg:`help patterns` for information about file patterns.
@@ -654,7 +654,7 @@
 return subset.filter(lambda r: _matchvalue(r),
  condrepr=('', rev))
 
-@predicate('date(interval)', safe=True)
+@predicate('date(interval)', safe=True, weight=10)
 def date(repo, subset, x):
 """Changesets within the interval, see :hg:`help dates`.
 """
@@ -664,7 +664,7 @@
 return subset.filter(lambda x: dm(repo[x].date()[0]),
  condrepr=('', ds))
 
-@predicate('desc(string)', safe=True)
+@predicate('desc(string)', safe=True, weight=10)
 def desc(repo, subset, x):
 """Search commit message for string. The match is case-insensitive.
 
@@ -722,7 +722,7 @@
 # Like ``descendants(set)`` but follows only the first parents.
 return _descendants(repo, subset, x, followfirst=True)
 
-@predicate('destination([set])', safe=True)
+@predicate('destination([set])', safe=True, weight=10)
 def destination(repo, subset, x):
 """Changesets that were created by a graft, transplant or rebase operation,
 with the given revisions specified as the source.  Omitting the optional 
set
@@ -891,7 +891,7 @@
 
 return subset & s
 
-@predicate('first(set, [n])', safe=True, takeorder=True)
+@predicate('first(set, [n])', safe=True, takeorder=True, weight=0)
 def first(repo,

D657: revset: move weight information to predicate

2017-09-18 Thread quark (Jun Wu) 
quark added a comment.


  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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco, krbullock
Cc: krbullock, yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-18 Thread krbullock (Kevin Bullock) 
krbullock requested changes to this revision.
krbullock added a comment.
This revision now requires changes to proceed.


  I think either expanding the docs or using constants would be fine, but the 
default weight looks like it's 1, not 10.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco, krbullock
Cc: krbullock, yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D657: revset: move weight information to predicate

2017-09-09 Thread Yuya Nishihara 
On Fri, 8 Sep 2017 18:24:44 +, 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-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-08 Thread phillco (Phil Cohen) 
phillco added a comment.


  Constants could pobably help make it more self-documenting.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco
Cc: yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-08 Thread quark (Jun Wu) 
quark added a comment.


  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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco
Cc: yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-08 Thread yuja (Yuya Nishihara) 
yuja added a comment.


  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?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco
Cc: yuja, phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-07 Thread quark (Jun Wu) 
quark added inline comments.

INLINE COMMENTS

> phillco wrote in revset.py:256
> I take it this dictionary was moved there because of the import direction?

Yes.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco
Cc: phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-07 Thread phillco (Phil Cohen) 
phillco accepted this revision.
phillco added a comment.


  lgtm

INLINE COMMENTS

> revset.py:256
>  #   x - argument in tree form
> -symbols = {}
> +symbols = revsetlang.symbols
>  

I take it this dictionary was moved there because of the import direction?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

To: quark, #hg-reviewers, phillco
Cc: phillco, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D657: revset: move weight information to predicate

2017-09-07 Thread quark (Jun Wu) 
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D657

AFFECTED FILES
  mercurial/registrar.py
  mercurial/revset.py
  mercurial/revsetlang.py

CHANGE DETAILS

diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -49,6 +49,8 @@
 
 keywords = {'and', 'or', 'not'}
 
+symbols = {}
+
 _quoteletters = {'"', "'"}
 _simpleopletters = set(pycompat.iterbytestr("()[]#:=,-|&+!~^%"))
 
@@ -441,21 +443,7 @@
 elif op == 'func':
 f = getsymbol(x[1])
 wa, ta = _optimize(x[2])
-if f in ('author', 'branch', 'closed', 'date', 'desc', 'file', 'grep',
- 'keyword', 'outgoing', 'user', 'destination'):
-w = 10 # slow
-elif f in ('modifies', 'adds', 'removes'):
-w = 30 # slower
-elif f == "contains":
-w = 100 # very slow
-elif f == "ancestor":
-w = 0.5
-elif f in ('reverse', 'limit', 'first', 'wdir', '_intlist'):
-w = 0
-elif f == "sort":
-w = 10 # assume most sorts look at changelog
-else:
-w = 1
+w = getattr(symbols.get(f), '_weight', 1)
 return w + wa, (op, x[1], ta)
 raise ValueError('invalid operator %r' % op)
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -253,7 +253,7 @@
 #   repo - current repository instance
 #   subset - of revisions to be examined
 #   x - argument in tree form
-symbols = {}
+symbols = revsetlang.symbols
 
 # symbols which can't be used for a DoS attack for any given input
 # (e.g. those which accept regexes as plain strings shouldn't be included)
@@ -276,7 +276,7 @@
 sourceset = getset(repo, fullreposet(repo), x)
 return subset & baseset([destutil.destmerge(repo, sourceset=sourceset)])
 
-@predicate('adds(pattern)', safe=True)
+@predicate('adds(pattern)', safe=True, weight=30)
 def adds(repo, subset, x):
 """Changesets that add a file matching pattern.
 
@@ -288,7 +288,7 @@
 pat = getstring(x, _("adds requires a pattern"))
 return checkstatus(repo, subset, pat, 1)
 
-@predicate('ancestor(*changeset)', safe=True)
+@predicate('ancestor(*changeset)', safe=True, weight=0.5)
 def ancestor(repo, subset, x):
 """A greatest common ancestor of the changesets.
 
@@ -394,7 +394,7 @@
 ps.add(r)
 return subset & ps
 
-@predicate('author(string)', safe=True)
+@predicate('author(string)', safe=True, weight=10)
 def author(repo, subset, x):
 """Alias for ``user(string)``.
 """
@@ -462,7 +462,7 @@
 bms -= {node.nullrev}
 return subset & bms
 
-@predicate('branch(string or set)', safe=True)
+@predicate('branch(string or set)', safe=True, weight=10)
 def branch(repo, subset, x):
 """
 All changesets belonging to the given branch or the branches of the given
@@ -595,16 +595,16 @@
 cs = _children(repo, subset, s)
 return subset & cs
 
-@predicate('closed()', safe=True)
+@predicate('closed()', safe=True, weight=10)
 def closed(repo, subset, x):
 """Changeset is closed.
 """
 # i18n: "closed" is a keyword
 getargs(x, 0, 0, _("closed takes no arguments"))
 return subset.filter(lambda r: repo[r].closesbranch(),
  condrepr='')
 
-@predicate('contains(pattern)')
+@predicate('contains(pattern)', weight=100)
 def contains(repo, subset, x):
 """The revision's manifest contains a file matching pattern (but might not
 modify it). See :hg:`help patterns` for information about file patterns.
@@ -654,7 +654,7 @@
 return subset.filter(lambda r: _matchvalue(r),
  condrepr=('', rev))
 
-@predicate('date(interval)', safe=True)
+@predicate('date(interval)', safe=True, weight=10)
 def date(repo, subset, x):
 """Changesets within the interval, see :hg:`help dates`.
 """
@@ -664,7 +664,7 @@
 return subset.filter(lambda x: dm(repo[x].date()[0]),
  condrepr=('', ds))
 
-@predicate('desc(string)', safe=True)
+@predicate('desc(string)', safe=True, weight=10)
 def desc(repo, subset, x):
 """Search commit message for string. The match is case-insensitive.
 
@@ -722,7 +722,7 @@
 # Like ``descendants(set)`` but follows only the first parents.
 return _descendants(repo, subset, x, followfirst=True)
 
-@predicate('destination([set])', safe=True)
+@predicate('destination([set])', safe=True, weight=10)
 def destination(repo, subset, x):
 """Changesets that were created by a graft, transplant or rebase operation,
 with the given revisions specified as the source.