Follow up on [JDK-7018422] - JavaAgent code always interpreted during initialization phase

[Severin Gehwolf]

Hi,

On Thu, 2021-01-28 at 14:37 +1000, David Holmes wrote:
> Hi,
> 
> On 28/01/2021 10:51 am, Nhat Nguyen wrote:
> > Hi,
> > 
> > I'm writing this email to follow up on JDK-7018422 [1] on jdk8u, where the JavaAgent code
> > is always interpreted during the initialization phase on jdk8.
> 
> Just FYI for changes to 8u you need to raise the issue on 
> jdk8u-dev at openjdk.java.net.

Adding jdk8u-dev to cc.

> 
> > For background, one of our tools is currently making use of a JavaAgent whose performance
> > is important. As part of a performance investigation, I discovered that the CompilerBroker is
> > not initialized by the time the premain method runs, therefore all compilation requests for
> > the JavaAgent are discarded.
> > 
> > Fortunately, I found JDK-7018422 which describes the exact issue that we are experiencing.
> > However, the ticket was closed as "Not an issue" because the initialization order is changed
> > with the introduction of the module system.
> > 
> > So I would like to ask the mailing list for opinions on whether a fix for this issue can be
> > considered for jdk8u. I have also attached a prototype fix [2] and would appreciate any
> > suggestions and comments as well.
> 
> You have to be extremely careful changing the initialization order as 
> there are many hidden inter-dependencies.

Yes, and we've been bitten by this before[i]. There is a significant
risk involved changing initialization order in stable jdk8u. The
reasons given here  don't strike me as a good enough reason to accept
such a patch. YMMV.

Andrew Haley, thoughts?

Thanks,
Severin

[i] https://bugs.openjdk.java.net/browse/JDK-8249158
    and https://bugs.openjdk.java.net/browse/JDK-8233197

> And as Serguei stated, that change may potentially allow JIT'ing to be 
> possible, but that doesn't mean it will actually occur.
> 
> Cheers,
> David
> -----
> 
> > 
> > [1]: https://bugs.openjdk.java.net/browse/JDK-7018422
> > [2]:
> > 
> > ---
> >   hotspot/src/share/vm/runtime/thread.cpp | 10 +++++-----
> >   1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hotspot/src/share/vm/runtime/thread.cpp
> > b/hotspot/src/share/vm/runtime/thread.cpp
> > index 330593acb3..3918f989cc 100644
> > --- a/hotspot/src/share/vm/runtime/thread.cpp
> > +++ b/hotspot/src/share/vm/runtime/thread.cpp
> > @@ -3634,11 +3634,6 @@ jint Threads::create_vm(JavaVMInitArgs*
> > args, bool* canTryAgain) {
> >       create_vm_init_libraries();
> >     }
> >   
> > -  // Notify JVMTI agents that VM initialization is complete - nop
> > if no agents.
> > -  JvmtiExport::post_vm_initialized();
> > -
> > -  JFR_ONLY(Jfr::on_vm_start();)
> > -
> >     if (CleanChunkPoolAsync) {
> >       Chunk::start_chunk_pool_cleaner_task();
> >     }
> > @@ -3648,6 +3643,11 @@ jint Threads::create_vm(JavaVMInitArgs*
> > args, bool* canTryAgain) {
> >     CompileBroker::compilation_init();
> >   #endif
> >   
> > +  // Notify JVMTI agents that VM initialization is complete - nop
> > if no agents.
> > +  JvmtiExport::post_vm_initialized();
> > +
> > +  JFR_ONLY(Jfr::on_vm_start();)
> > +
> >     if (EnableInvokeDynamic) {
> >       // Pre-initialize some JSR292 core classes to avoid deadlock
> > during class loading.
> >       // It is done after compilers are initialized, because
> > otherwise compilations of
> > --
> > 
>