[15] RFR: 8193210: [JVMCI/Graal] add JFR compiler phase/inlining events

Jamsheed C M jamsheed.c.m at oracle.com
Fri Mar 6 17:04:47 UTC 2020


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-jfr-dev mailing list