RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field access event requests at runtime

[Doerr, Martin]

Hi David and Erik,

I've tried to add the capability "can_generate_field_access_events" during live phase and got "AddCapabilities failed with error 98" which is "JVMTI_ERROR_NOT_AVAILABLE". So hotspot does not support switching it on during live phase.

Hotspot initializes "can_generate_field_modification_events" during "init_onload_solo_capabilities". As the name tells, it is implemented as an "onload" capability.

So the VM works as expected with and without my change.

Can I add you as reviewers?
If yes, which parts did you review (x86, SPARC, shared code)?

Thanks and best regards,
Martin


> -----Original Message-----
> From: Doerr, Martin
> Sent: Freitag, 19. Juli 2019 13:11
> To: David Holmes <david.holmes at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net; serviceability-dev at openjdk.java.net; Erik Osterlund
> <erik.osterlund at oracle.com>
> Subject: RE: RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field access
> event requests at runtime
> 
> Hi David,
> 
> thanks for elaborating on the capability enablement.
> With respect to "AddCapabilities", I've only found "Typically this function is
> used in the OnLoad function. Some virtual machines may allow a limited set
> of capabilities to be added in the live phase." in the spec [1].
> I don't know which ones are supposed to be part of this "limited set of
> capabilities".
> As you already explained, adding the capability for field access events in the
> live phase does obviously not work for hotspot.
> The interpreter has the same issue.
> 
> Best regards,
> Martin
> 
> 
> [1]
> https://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#AddCapa
> bilities
> 
> 
> > -----Original Message-----
> > From: David Holmes <david.holmes at oracle.com>
> > Sent: Freitag, 19. Juli 2019 02:30
> > To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> > dev at openjdk.java.net; serviceability-dev at openjdk.java.net; Erik
> Osterlund
> > <erik.osterlund at oracle.com>
> > Subject: Re: RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field
> access
> > event requests at runtime
> >
> > Hi Martin,
> >
> > On 18/07/2019 8:01 pm, Doerr, Martin wrote:
> > > Hi David and Erik,
> > >
> > > thank you for looking at my proposal.
> > >
> > >> If you try to use fast field accessors when you have to post the field
> > >> access event then how can you safely go off into a JVM TI event callback
> > ??
> > >
> > > We speculatively load the field and check afterwards if we can use this
> > loaded value.
> > > It is safe to use it if there was no safepoint and no JVMTI event was
> > requested.
> > > Otherwise, we simply discard the (possibly) loaded value and load it again
> > in the slow path where we do all the synchronization and event posting.
> >
> > Thanks for clarifying for me. That is all fine then.
> >
> > The dynamics of this still concern me, but those concerns are also
> > present in the existing code. Currently we don't use the quick accessors
> > if JvmtiExport::can_post_field_access() is true during VM startup - this
> > is a one-of initialization check that sets the use of fast accessors for
> > the lifetime of the JVM. But that is set between the early-start and
> > start VM events, before the live-phase. But AFAICS the capability for
> > can_post_field_access can be set or cleared dynamically during the live
> > phase, thus invalidating the original decision on whether to use fast
> > accessors or not. With your changes the state of can_post_field_access
> > is still captured during VM initialization so again the decision to
> > check for a field access watch is hard-wired for the lifetime of the VM.
> > But once installed that check allows for use of the fast-path if no
> > actual watches are set - which is the whole point of this enhancement.
> > So the issue with both old and new code is that if the capability is not
> > present at VM startup the VM will be configured to always use the fast
> > path, even if the capability (and field access watches) are added later.
> >
> > Thanks,
> > David
> >
> > > @Erik:
> > > Thanks for your proposal to change the function pointers. I'll look into
> that.
> > >
> > > Best regards,
> > > Martin
> > >
> > >
> > >> -----Original Message-----
> > >> From: David Holmes <david.holmes at oracle.com>
> > >> Sent: Donnerstag, 18. Juli 2019 06:39
> > >> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> > >> dev at openjdk.java.net; serviceability-dev at openjdk.java.net
> > >> Subject: Re: RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field
> > access
> > >> event requests at runtime
> > >>
> > >> Hi Martin,
> > >>
> > >> I need to think about this some more. A critical property of the fast
> > >> field accessors are that they are trivial and completely safe. They are
> > >> complicated by the need to check if a GC may have happened while we
> > >> directly read the field.
> > >>
> > >> If you try to use fast field accessors when you have to post the field
> > >> access event then how can you safely go off into a JVM TI event callback
> > ??
> > >>
> > >> Thanks,
> > >> David
> > >>
> > >> On 16/07/2019 11:31 pm, Doerr, Martin wrote:
> > >>> Hi,
> > >>>
> > >>> the current implementation of FastJNIAccessors ignores the flag -
> > >> XX:+UseFastJNIAccessors when the JVMTI capability
> > >> "can_post_field_access" is enabled.
> > >>> This is an unnecessary restriction which makes field accesses
> > >> (Get<Type>Field) from native code slower when a JVMTI agent is
> > attached
> > >> which enables this capability.
> > >>> A better implementation would check at runtime if an agent actually
> > wants
> > >> to receive field access events.
> > >>>
> > >>> Note that the bytecode interpreter already uses this better
> > >> implementation by checking if field access watch events were
> requested
> > >> (JvmtiExport::_field_access_count != 0).
> > >>>
> > >>> I have implemented such a runtime check on all platforms which
> > currently
> > >> support FastJNIAccessors.
> > >>>
> > >>> My new jtreg test runtime/jni/FastGetField/FastGetField.java contains
> a
> > >> micro benchmark:
> > >>> test-
> > >>
> >
> support/jtreg_test_hotspot_jtreg_runtime_jni_FastGetField/runtime/jni/Fa
> > >> stGetField/FastGetField.jtr
> > >>> shows the duration of 10000 iterations with and without
> > >> UseFastJNIAccessors (JVMTI agent gets attached in both runs).
> > >>> My Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz needed 4.7ms with
> > >> FastJNIAccessors and 11.2ms without it.
> > >>>
> > >>> Webrev:
> > >>>
> > >>
> >
> http://cr.openjdk.java.net/~mdoerr/8227680_FastJNIAccessors/webrev.00/
> > >>>
> > >>> We have run the test on 64 bit x86 platforms, SPARC and aarch64.
> > >>> (FastJNIAccessors are not yet available on PPC64 and s390. I'll
> contribute
> > >> them later.)
> > >>> My webrev contains 32 bit implementations for x86 and arm, but
> > >> completely untested. It'd be great if somebody could volunteer to
> review
> > >> and test these platforms.
> > >>>
> > >>> Please review.
> > >>>
> > >>> Best regards,
> > >>> Martin
> > >>>