[15] RFR: 8193210: [JVMCI/Graal] add JFR compiler phase/inlining events
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Mar 24 22:05:51 UTC 2020
Compiler changes review.
You don't need to put all #if INCLUDE_JFR into compilerEvent.cpp.
First you can exclude this file from compilation when JFR is not enabled:
make/hotspot/lib/JvmFeatures.gmk Tue Mar 24 14:42:11 2020 -0700
@@ -173,6 +173,7 @@
ifneq ($(call check-jvm-feature, jfr), true)
JVM_CFLAGS_FEATURES += -DINCLUDE_JFR=0
JVM_EXCLUDE_PATTERNS += jfr
+ JVM_EXCLUDE_FILES += compilerEvent.cpp
endif
You can use NOT_JFR_RETURN_(x) macro in compilerEvent.hpp (you used it already in ticksNow()):
static int register_phases(GrowableArray<const char*>* new_phases) NOT_JFR_RETURN_(-1);
You can also add new macro NOT_JFR_RETURN for cases when methods don't return.
Other changes in compiler changes look fine to me.
Thanks,
Vladimir
On 3/23/20 11:06 AM, Jamsheed C M wrote:
> 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.ci/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/compiler/compilerEvent.hpp.html>/
>>> inline.hpp
>>> <http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/compiler/compilerEvent.inline.hpp.html>/ ..cpp
>>> <http://cr.openjdk.java.net/~jcm/8193210/webrev/src/hotspot/share/compiler/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/classes/jdk/jfr/internal/consumer/ChunkParser.java.udiff.html
>>>
>>>
More information about the hotspot-compiler-dev
mailing list