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

[David Holmes]

Hi Martin,

On 23/07/2019 1:39 am, Doerr, Martin wrote:
> 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.

Okay - thanks for verifying that.

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

I reviewed x86, sparc and shared code.

Thanks,
David

> 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
>>>>>>