D451: revset: remove order information from tree

[yuja (Yuya Nishihara)]

yuja added a comment.


  First, I'm tired of discussing this. Perhaps, you would be the same (guessing
  from the initial reply.) I don't think this should be a blocker of your previous
  series which optimizes `draft() ...` something.
  
  If you want to move things forward, please split non-controversial parts,
  which I think are:
  
  - remove "order" from parsed tree, use `_flipand()` instead
  - make `subset` argument optional
  
  Alternatively, I could send my build/matchtree patch without respecting to
  this series to unblock your original patch.
  
  > I just got a new idea: Wrap those 3-argument predicate in a function that
  >  does an extra `sort` if it's defineorder [1]. That seems to solve things
  >  cleanly.
  
  I don't think it's clean, but yeah doable. If I had to take this series, that would
  be the safest workaround for old code.
  
  >   mozilla-central % hg.old perfrevset 'sort(public() & 1:3)'
  >   ! wall 0.193297 comb 0.190000 user 0.190000 sys 0.000000 (best of 51)
  >   mozilla-central % hg.new perfrevset 'sort(public() & 1:3)'
  >   ! wall 0.000188 comb 0.000000 user 0.000000 sys 0.000000 (best of 13504)
  >    
  > 
  > So I think `anyorder` definitely has value (not that much to a revset expert though).
  
  In this example, `public() & 1:3` could be fully optimized to `anyorder`
  without the help of `_flipand()`. It's obvious that the order of `public() & 1:3`
  doesn't matter.
  
  >> We can apply `anyorder` optimization to inner queries even if we decided to not
  >>  optimize `_flipand()`. For example, think `ancestors(complex_query)`, where
  >>  `complex_query` can be computed in `anyorder` constraint because we can be
  >>  sure the order of head revisions doesn't matter. OTOH, `_flipand(x, y)` is hard
  >>  because `x` is passed to arbitrary sub-expression `y`.
  > 
  > I hope the word "hard" here could suggest that the new code does make things //simpler// at least in this case.
  
  I think it's "simpler" at the cost of bringing the "order" concept everywhere.
  
  > We can say revsets taking `N` revs returning `O(N)` revs would maintain
  >  the order, and not for other revsets. That'd be clearly defined.
  >  But I don't feel strong. We can keep the existing behavior here.
  
  Yep. I don't want to think about the complexity to determine if a revset
  predicate may define its own order.
  
  > Maybe. But if you insist `defineorder` does not always need to "define" order.
  >  I hope I can rename it to `maybedefineorder`, which will make it less confusing
  >  for developers.
  
  Seems good.
  
  > That said, I still prefer strong "define"s. It seems Martin
  >  also likes it.
  
  I guess he liked it since the new registrar would get rid of the `subset`
  argument. So do I in that regard.
  
  > I think it's simpler because both developers and end-users
  >  won't need to worry about the "weak define" concept.
  
  Users and (most) developers don't have to think about it in either case.
  Almost all revsets should be in revision order.

REPOSITORY
  rHG Mercurial

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

To: quark, #hg-reviewers
Cc: martinvonz, yuja, mercurial-devel