On Wed, 4 Sep 2024 14:18:09 GMT, Claes Redestad <redes...@openjdk.org> wrote:

>>> LGTM
>>> 
>>> AFAICT before this we'd only call `DirectCodeBuilder::labelToBci` once per 
>>> label, but now we'll do so roughly `2*n*log(n) + n` times. I would assume 
>>> getting rid of the `TreeMap` and `Integer` key allocations more than makes 
>>> up for this, though. Do we have any JMH tests where `writeFrames` is a 
>>> significant contributor we could check?
>> 
>> My expectation was that the short path of `DirectCodeBuilder` (where 
>> `context == this` and thus we directly return `lab.getBCI()`, which is a 
>> simple field getter) would be used in most, if not all, cases. If so, then 
>> this amounts to a fairly minimal and direct code path, thus I approached 
>> this as being an "obvious" (as opposed to theoretical) improvement.
>
>> If so, then this amounts to a fairly minimal and direct code path, thus I 
>> approached this as being an "obvious" (as opposed to theoretical) 
>> improvement.
> 
> I agree that it looks like the typical path is trivial, but I don't have the 
> full picture to understand when we might do more complicated work here. It 
> would be nice to have some reasonably realistic benchmark to lean on.

@cl4es would love to see a benchmark, but from CF API developer's perspective, 
this change alone should be good enough. TreeMap is like a linked list, aside 
from the reduction of allocations, this array also has a locality advantage; so 
things should be fine. Except this won't manifest itself much, maybe a tiny 
little bit for ProxyGenerator.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20841#issuecomment-2329542210

Reply via email to