On Wed, 4 Sep 2024 14:12:06 GMT, David M. Lloyd <d...@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?
>
>> 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.

@dmlloyd I think we might need a benchmark that uses 
`ClassFile.of(StackMapsOption.DROP_STACK_MAPS)` to build a class with method 
with code, and supply the stack maps to generate. That is the best way to 
measure the gains from this patch. (Something like what we do in 
ProxyGenerator, but proxy generation has too many uncertainties)

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

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

Reply via email to