Improving %ld substiution in `repo.revs`

[Boris FELD]

On 17/12/2018 14:36, Yuya Nishihara wrote:
> On Mon, 17 Dec 2018 12:38:17 +0000, Boris FELD wrote:
>> It is possible to passvarious substitution when passing revset to
>> `repo.revs` (eg: repo.revs("heads(%ld)", [4, 5, 6]). 
>>
>> The current implementation handle this in a quick and dirty way. First a
>> new revset string is build:
>>
>>
>>  * code: repo.revs("heads(%ld)", [4, 5, 6])
>>  * transformed into (repo.revs("heads(_intlist(4, 5, 6))")
>>  * then revset parser build a syntax tree
>>  * _intlist turns its argument into a baseset
>>
>> This is clever and simple, it works well for a small number of revision.
>> However, when the set of revision growth, this becomes ineffective. For
>> example, discovery might call revset where %ld is replace by hundred of
>> thousands of revisions. The cost of serializing and serializing them
>> to/from string become significant.
> To avoid formatting/parsing overheads, we could directly build a parse tree
> and feed it to revset.makematcher(). It will be somewhat similar to alias
> expansion.

The current API is very easy to use and I would rather keep it that way.
Having developer choose between API simplicity and speed seems sub-optimal.

What does your proposal look like in practice API wise?
>
> Another option is to add a private revset function that returns a
> precomputed set. For example:
>
> https://bitbucket.org/yuja/thg-work/commits/ee342febb5f445de63040f69ca1a6657916a4064#Ltortoisehg/util/preparedrevset.pyT123
>
> Unlike templater, revset has no context variable. So I had to hack the repo
> instance in the example above, which isn't nice for your use case.
Could we have this done "automatically" with repo.revs? where %ld turned
into special local revsets?
>
>> In addition, the serialization looses any possible optimization from the
>> source of revisions. For example, if we feed a smartset to `%ld` it
>> could be nice to keep it a smartset.
> This wouldn't be simple if we want to implement it cleanly. Maybe we can
> embed a smartset in a parse tree, as say (smartset <smartset>) node, but
> a smartset is mutable and it can be sorted/reordered while evaluating a
> revset. That's probably okay in many cases, but can lead to surprising
> results.
Ha, good point, maybe we need some sort of light smartset copy.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel