[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RE: RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field access event requests at runtime
From:       "Doerr, Martin" <martin.doerr () sap ! com>
Date:       2019-07-29 9:45:41
Message-ID: AM6PR02MB4788D47A66D11BE6B7EF7F939ADD0 () AM6PR02MB4788 ! eurprd02 ! prod ! outlook ! com
[Download RAW message or body]

Hi Boris,

thanks, I've updated the BUFFER_SIZE in place.

Seems like all platform implementations have been reviewed.
So I'll push this version if there are no objections.

Thanks everyone for reviewing!
Best regards,
Martin


> -----Original Message-----
> From: Boris Ulasevich <boris.ulasevich@bell-sw.com>
> Sent: Freitag, 26. Juli 2019 18:18
> To: Doerr, Martin <martin.doerr@sap.com>
> Cc: hotspot-runtime-dev@openjdk.java.net; serviceability-
> dev@openjdk.java.net
> Subject: Re: RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field access
> event requests at runtime
> 
> Hi Martin,
> 
> The webrev.02 change works good if we increase BUFFER_SIZE. Current
> change gives "BUFFER_SIZE too small" assertion. I propose to change
> BUFFER_SIZE value to 120, it works Ok then.
> 
> glad to help you,
> regards,
> Boris
> 
> On 26.07.2019 16:59, Doerr, Martin wrote:
> > Hi Boris,
> >
> > thank you very much for testing.
> >
> > Unfortunately, arm 32 was also affected by the issue Erik has found for
> aarch64:
> > We need a little stronger memory barriers to support accessing volatile
> fields with correct ordering semantics.
> >
> > I've updated that in the current webrev already:
> >
> http://cr.openjdk.java.net/~mdoerr/8227680_FastJNIAccessors/webrev.02/
> >
> > I'm using
> membar(MacroAssembler::Membar_mask_bits(MacroAssembler::LoadLoad
> | MacroAssembler::LoadStore), Rtmp2), now.
> > I've already used a cross build to check that it compiles, but I haven't run it.
> > I believe this membar doesn't have a significant performance impact.
> >
> > Would be great if you could take a look and test that, too.
> >
> > Thanks and best regards,
> > Martin
> >
> >
> >> -----Original Message-----
> >> From: Boris Ulasevich <boris.ulasevich@bell-sw.com>
> >> Sent: Freitag, 26. Juli 2019 12:50
> >> To: Doerr, Martin <martin.doerr@sap.com>
> >> Cc: hotspot-runtime-dev@openjdk.java.net; serviceability-
> >> dev@openjdk.java.net
> >> Subject: Re: RFR(M): 8227680: FastJNIAccessors: Check for JVMTI field
> access
> >> event requests at runtime
> >>
> >> Hi Martin,
> >>
> >> Your change works Ok on arm32 with the minor correction. See the patch
> >> attached.
> >>
> >> thanks,
> >> Boris
> >>
> >> On 16.07.2019 16:31, 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
> >>>

[prev in list] [next in list] [prev in thread] [next in thread]