[15] RFR: 8193210: [JVMCI/Graal] add JFR compiler phase/inlining events
Markus Gronlund
markus.gronlund at oracle.com
Wed Mar 25 12:42:24 UTC 2020
Hi Jamsheed,
Still looks good.
Thank you
Markus
-----Original Message-----
From: Jamsheed C M
Sent: den 23 mars 2020 19:07
To: Markus Gronlund <markus.gronlund at oracle.com>; hotspot-jfr-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net; Douglas Simon <doug.simon at oracle.com>; Vladimir Kozlov <vladimir.kozlov at oracle.com>
Subject: Re: [15] RFR: 8193210: [JVMCI/Graal] add JFR compiler phase/inlining events
Hi all,
There were some changes to the requirement. JVMCI phase name registration logic likes it to be completely dynamic(or lazily added) making old assumptions and implementation invalid(i.e. The serializer re-registration logic was very wasteful at startup for totally lazily available java phase names). So, the phase names registration logic is changed for better incremental registration support (see CompilerEvent::PhaseEvent::register_phases).
JVMCI CompilerPhaseEvent uses JFR C++ event definition EventCompilerPhase(defined in src/share/jfr/metadata/metadata.xml). This required CompilerPhaseType to be extended and used for JVMCI Compiler phase names that are lazily added to the VM, more on it below.
Changes to CompilerPhaseType.
JFR type CompilerPhaseType is extended for JVMCI use, new mapping is maintained as growable array phase_names (in compiler/compilerEvent.cpp).
c2 mapping for the type is located in opto/phasetype.hpp. It is explicit mapping, so if it is used, it should be appended to phase_names before any other mapping.
JVMCI phase name are appended to phase_names lazily(when phases are first used by the jvmci compiler). Its phaseToId version is located at jdk.vm.ci.hotspot.JFR.phaseToId.
Changes to its serializer[1].
CompilerPhaseTypeConstant serializer requires changes for handling dynamic entries.
The types managed by the JfrSerializer framework are usually static by nature, but there were exceptions like JfrNetworkUtilization(where the type values are added lazily/dynamic).
Although adding new entries to the type lazily and serializing it for every new chunk created later was easy to do with current setup, there was no external support available for serializing the new entries for the current chunk (i.e. JVMCI use case was similar to JfrNetworkUtilization use case, but JfrCheckpointWriter was internal feature and was not available outside JFR module)
So the serializer re-registration feature was used for serializing it for the current chunk in the webrev[2], but it was wasteful at startup.
JFR team provided some improvement over this, and made JfrCheckpointWriter available outside JFR module. This was possible by the improved flush logic provided by Markus.
The new improved phase name registration logic is available at webrev.1 http://cr.openjdk.java.net/~jcm/8193210/webrev.1/
Files changed in webrev.1
src/hotspot/share/compiler/compilerEvent.hpp
src/hotspot/share/compiler/compilerEvent.cpp
src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
src/hotspot/share/compiler/compileBroker.cpp
JFR changes (Flush logic improvement provided my Markus) src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp
src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp
src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.hpp
src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp
Other changes (Function name changes)
http://cr.openjdk.java.net/~jcm/8193210/webrev.1/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp.udiff.html
http://cr.openjdk.java.net/~jcm/8193210/webrev.1/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java.udiff.html
tracingTimeNow => ticksNow
writeEventCompilerPhase => notifyCompilerPhaseEvent writeEventCompilerInlining => notifyCompilerInliningEvent
http://cr.openjdk.java.net/~jcm/8193210/webrev.1/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/JFR.java.html
registerCompilerPhases => registerPhases Also, added logic for duplicate removal for non regular bulk registration use case(not properly synchronized, so there can be still duplicates in registrationl).
Thanks a lot Markus for the continuous support provided for this work.
Please review.
Best regards,
Jamsheed
[1] Serializers for types(subclasses of JfrSerializer) serializes type on its registration, once registered, type is serliazed for every new chunks that is created later.
[2] http://cr.openjdk.java.net/~jcm/8193210/webrev/
[3] http://cr.openjdk.java.net/~jcm/8193210/webrev.1/
On 06/03/2020 22:34, Jamsheed C M wrote:
> Hi Markus,
>
> On 06/03/2020 21:32, Markus Gronlund wrote:
>>
>> Hi Jamsheed,
>>
>> Looks good.
>>
> Thank you for the review.
>>
>> Good work in getting this integration in place.
>>
> Sure, it wouldn't have been easy without the quick jfr serializer
> (Re)registration support provided by you. Thanks for it again.
>
> Best regards,
>
> Jamsheed
>
>> Thanks
>>
>> Markus
>>
>> *From:* Jamsheed C M
>> *Sent:* den 29 februari 2020 08:28
>> *To:* hotspot-jfr-dev at openjdk.java.net;
>> hotspot-compiler-dev at openjdk.java.net; Markus Gronlund
>> <markus.gronlund at oracle.com>; Doug Simon <doug.simon at oracle.com>;
>> Vladimir Kozlov <vladimir.kozlov at oracle.com>
>> *Subject:* Re: [15] RFR: 8193210: [JVMCI/Graal] add JFR compiler
>> phase/inlining events
>>
>> Hi all,
>>
>> i did some more updates to webrev.
>>
>> 1) used short names for CompilerEvent::PhaseEvent::post.. shortened
>> to CompilerEvent::PhaseEvent::post
>>
>> 2) added detailed comments for
>> CompilerEvent::PhaseEvent::register_phases. There is some implicit
>> assumption at usage site, like c2 is always registered first. added
>> comment to make it clear, and made registration final if jvmci is
>> not included.
>>
>> Best regards,
>>
>> Jamsheed
>>
>> On 28/02/2020 19:52, Jamsheed C M wrote:
>>
>> also removed compilerEvent.inline.hpp.. and made most of guarded
>> profiling code out-of-line.
>>
>> updated webrev in place.
>>
>> Best regards,
>>
>> Jamsheed
>>
>> On 28/02/2020 10:38, Jamsheed C M wrote:
>>
>> Hi,
>>
>> missed newly added Deoptimization event test. also
>> TestJfrJavaBase.java test**
>>
>> modified deopt test for jvmci(graal) use case,
>> TestJfrJavaBase
>> is not relevant as compiler is not available for the test.
>>
>> updated webrev in place.
>>
>> Best regards,
>>
>> Jamsheed
>>
>> On 26/02/2020 14:19, Jamsheed C M wrote:
>>
>> Hi,
>>
>> i messed up with order of includes, got undetected due to
>> PCH. corrected it, updated webrev in place.
>>
>> corrected the title.
>>
>> Best regards,
>>
>> Jamsheed
>>
>> On 26/02/2020 07:35, Jamsheed C M wrote:
>>
>> Hi,
>>
>> This enhancement adds jvmci supports that is required
>> for extending existing phase/inlining events support
>> in graal compilers.
>>
>> During this enhancement all jfr events related codes
>> are moved to compiler/compilerEvent.[.hpp,
>> inline.hpp,
>> .cpp]
>>
>> Graal use these events via jvmci JFR helper class,
>> which uses c2v interface to post and get information
>> from hotspot
>>
>> Overview of functionalities exposed by JFR helper
>> class
>> <http://cr.openjdk.java.net/~jcm/8193210/webrev/src/jdk.internal.vm.c
>> i/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/JFR.java.html
>> >
>>
>> Ticks, for getting current jfr counter/ time stamp.
>> used for getting start time of events by Graal compiler.
>>
>> CompilerPhaseEvent for posting phase event to hotspot
>> JFR.
>>
>> CompilerInlininingEvent for posting inlininig events.
>>
>> CompilerPhaseEvent extends and use
>> TYPE_COMPILERPHASETYPE content type for phase events,
>> this is achieved using reregistration support added
>> by
>> Markus [1]
>>
>> CompilerEvent::PhaseType::register_phases is used for
>> registering phases. This function is usually called
>> on
>> compiler singleton object creation. It is also called
>> lazily if phases are not known beforehand.
>>
>> The event generation codes in c1/c1_GraphBuilder.cpp,
>> opto/bytecodeInfo.cpp, opto/compile.hpp uses common
>> functions exposed by compiler/compilerEvent.hpp
>> <http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/com
>> piler/compilerEvent.hpp.html>/
>> inline.hpp
>> <http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/com
>> piler/compilerEvent.inline.hpp.html>/ ..cpp
>> <http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/com
>> piler/compilerEvent.cpp.html>
>>
>> webrev:
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/
>>
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8193210
>>
>> Testing: ran jdk/jfr in my local machine. mach5 run
>> created, links in jbs.
>>
>> Please review,
>>
>> Best regards,
>>
>> Jamsheed
>>
>> [1]
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/jfr/
>> recorder/checkpoint/jfrCheckpointManager.cpp.udiff.html
>>
>>
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/jfr/
>> recorder/checkpoint/jfrCheckpointWriter.hpp.udiff.html
>>
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/jfr/
>> recorder/checkpoint/types/jfrTypeManager.cpp.udiff.html
>>
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/jfr/
>> recorder/checkpoint/types/jfrTypeManager.hpp.udiff.html
>>
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/jfr/
>> utilities/jfrTypes.hpp.udiff.html
>>
>> http://cr.openjdk.java.net/~jcm/8193210/webrev/src/jdk.jfr/share/clas
>> ses/jdk/jfr/internal/consumer/ChunkParser.java.udiff.html
>>
More information about the hotspot-compiler-dev
mailing list