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@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel