On Tue, 27 Sep 2022 06:48:50 GMT, David Holmes <dhol...@openjdk.org> wrote:

> Good find! Looks good! A couple of queries at this stage.
> 
> Thanks.
>
Thanks for looking at this David.

> src/hotspot/share/classfile/javaClasses.cpp line 2004:
> 
>> 2002:       const bool skip_hidden = !ShowHiddenFrames;
>> 2003: 
>> 2004:       // Pick some initial length
> 
> The comment should at least hint at there being some reasonable reason for 
> choosing the value that follows. :)

How about: "Pick minimum length that will cover most cases"?

> src/hotspot/share/classfile/javaClasses.cpp line 2008:
> 
>> 2006:       _methods = new (ResourceObj::C_HEAP, mtInternal) 
>> GrowableArray<Method*>(init_length, mtInternal);
>> 2007:       _bcis = new (ResourceObj::C_HEAP, mtInternal) 
>> GrowableArray<int>(init_length, mtInternal);
>> 2008: 
> 
> Couldn't you just do this in the constructor? I'm not clear if there is a 
> subtle reason for needing lazy-init as well as moving to the C_Heap.

Yes, this could have been done in the constructor too. But since there are 
additional checks in the closure that could fail I move the allocation here to 
avoid unnecessary allocation/deallocation. The allocation still needs to be 
done in the C_Heap in case the target executes the handshake. Otherwise if the 
target allocates the arrays in its resource area they could be deallocated by 
the time the requester tries to access them after the handshake.

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

PR: https://git.openjdk.org/jdk/pull/10424

Reply via email to