[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#62368: 29.0.60; Evaluating predicates before creating captured nodes
From: |
Yuan Fu |
Subject: |
bug#62368: 29.0.60; Evaluating predicates before creating captured nodes in treesit-query-capture |
Date: |
Wed, 22 Mar 2023 20:16:14 -0700 |
> 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