Improving %ld substiution in `repo.revs`

[Boris FELD]

On 20/12/2018 14:23, Yuya Nishihara wrote:
> On Thu, 20 Dec 2018 10:27:32 +0100, Boris FELD wrote:
>> 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?
> repo.revs("heads(%ld)", [4, 5, 6])
>   rewrite "heads(%ld)" as e.g. "heads($0)" so that it can be parsed as alias
>   tree = parse("heads($0)")
>   replace the placeholder arguments with e.g. ('intlist', 4, 5, 6)
>   analyze, optimize, etc. the tree
>   return revset.makematcher(tree)
This looks good, we'll dig in that direction.