RFR: 8241361: ZGC: Implement memory related JFR events

[Per Liden]

Hi,

On 3/20/20 2:43 PM, Stefan Karlsson wrote:
> Hi all,
> 
> Please review this patch to add some memory related JFR events to ZGC.
> 
> https://cr.openjdk.java.net/~stefank/8241361/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8241361

Nice! Looks good overall, a few minor things:


src/hotspot/share/gc/z/zPageAllocator.cpp
-----------------------------------------

* Instead of:

   #include "jfrfiles/jfrEventClasses.hpp"

I think we usually do:

   #include "jfr/jfrEvents.hpp"


* I could live without the whitespace you added on line 631.

  630   ZPageCacheFlushForAllocationClosure cl(requested);
  631
  632   const size_t flushed = flush_cache(&cl, true /* for_allocation


* I think cl->_flushed user here:

  604   event.commit(cl->_requested, cl->_flushed, for_allocation);

should instead just be:

  604   event.commit(cl->_requested, flushed, for_allocation);

Right?


src/hotspot/share/gc/z/zPageCache.hpp
-------------------------------------

Instead of:

   friend class ZPageAllocator;

add a getter for requested()?


src/hotspot/share/gc/z/zRelocationSetSelector.cpp
-------------------------------------------------

* Same here, instead of:

   #include "jfrfiles/jfrEventClasses.hpp"

I think we should do:

   #include "jfr/jfrEvents.hpp"


* You don't think we should use ZPageTypeType that you introduced, and 
send three different ZRelocationSet events, one for each page type? 
Shouldn't this event also be timed, and sent from within 
ZRelocationSetSelectorGroup::select()?


src/hotspot/share/gc/z/zTracer.cpp
----------------------------------

   43     writer.write("small");
   44     writer.write_key(ZPageTypeMedium);
   45     writer.write("medium");
   46     writer.write_key(ZPageTypeLarge);
   47     writer.write("large");

How about "Small", "Medium" and "Large"? I could only find one other 
place (in jfrStackTraceRepository.cpp) where names were given, and those 
start with a capital letter.

cheers,
Per


> 
> Added events:
> 
> ZAllocationStall - Record when we run out of heap memory and the Java 
> threads stall, waiting for the GC to free up memory.
> 
> ZPageAllocation - Updated the existing event to also record the duration 
> of the event. Updated the event to only be reported if the allocation 
> takes longer than 1 ms.
> 
> ZPageCacheFlush - Record when the page cache needs to be flushed. This 
> usually happens when we run out of a specific page size and have to 
> detach the physical and virtual memory to materialize a new ZPage. We 
> also flush pages when we uncommit memory.
> 
> ZRelocationSet - Record information about the selected relocation set.
> 
> ZUncommit - Record when we uncommit and hand back memory to the OS.
> 
> The patch also contains some small cosmetic changes to existing events, 
> whitespace fixes.