RFR: 8292059: Do not inline InstanceKlass::allocate_instance() [v2]

Coleen Phillimore coleenp at openjdk.org
Fri Mar 3 16:57:20 UTC 2023


On Fri, 3 Mar 2023 11:53:48 GMT, Afshin Zafari <duke at openjdk.org> wrote:

>> The inline and not-inline versions of the method is stress tested to compare the performance difference. The statistics are drawn in the following charts. The vertical axis is in milliseconds.
>> 
>> ![chart (2)](https://user-images.githubusercontent.com/4697012/221848555-2884313e-9d26-41c9-a265-3f1ce295b17b.png)
>> 
>> ![chart (3)](https://user-images.githubusercontent.com/4697012/221863810-94118677-b4af-468f-90c6-5ea365ae3588.png)
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8292059: Do not inline InstanceKlass::allocate_instance()

I have one suggested change but you should point out that the parameter change is an optimization that replaces some of the inlined benefits.

src/hotspot/share/oops/instanceKlass.cpp line 1390:

> 1388: }
> 1389: 
> 1390: instanceOop InstanceKlass::allocate_instance(InstanceKlass* ik, TRAPS) {

Should this have a null check on ik, like in the inlined function that it replaced?

src/hotspot/share/prims/jvmtiEnvBase.cpp line 28:

> 26: #include "classfile/classLoaderDataGraph.hpp"
> 27: #include "classfile/javaClasses.inline.hpp"
> 28: #include "classfile/vmSymbols.hpp"

I assume this is a transitive include, so good to put it in the file where it's used.

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

Changes requested by coleenp (Reviewer).

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


More information about the hotspot-dev mailing list