bug#62368: 29.0.60; Evaluating predicates before creating captured nodes in treesit-query-capture

[Yuan Fu]

> On Mar 22, 2023, at 5:42 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> Hi Yuan!
> 
> On 22/03/2023 06:49, Yuan Fu wrote:
>> X-Debbugs-CC:dgutov@yandex.ru
>> Dmitry, when you have time, could you try your benchmark in bug#60953
>> with this patch? I made predicates evaluate before we create any nodes,
>> so #equal and #match should be more efficient now, when there are a lot
>> of rejections. In the same time #pred is made slightly worst since they
>> now create a lisp node and discard it. (But this can be fixed with a
>> little more complexity.)
> 
> Thank you, I was curious what would the improvement be if we could delay 
> allocation of node structures until :match is checked.
> 
> But for my benchmark the difference is on the order of 4-5%. It seems we are 
> scraping the barrel in terms of improving allocations/reducing GC because 
> according to 'benchmark-run', where the whole run of a 100 iterations of the 
> scenario takes ~1.1s, the time spent in GC is 0.150s. And the improved 
> version takes like 1.04s, with 0.1s in GC.
> 
> So if you ask me, I think I'd prefer to hold off on applying this patch until 
> we either find scenarios where the improvement is more significant, or we 
> find and eliminate some other bigger bottleneck first, after which these 5% 
> grow to become 10-20% or more, of remaining runtime. The current approach is 
> pretty Lisp-y, so I kind of like it.
> 
> And there's the issue of #pred, of course, which which could swing the 
> difference in the other direction (I didn't test any code which uses it).
> 
> We could also try a smaller change: where the initial list of conses for 
> result is build with capture_id's in car's, and then substituted with 
> capture_name if the predicates all match. Then tthe treesit_node 
> pseudovectors would still be created eagerly, though.

Thank you very much! Yeah, it doesn’t seem to worth it. I guess we can keep 
this in our back sleeve for now ;-)

I think using symbols is fine for now, since I don’t think it would make much 
difference.

Yuan