RFR: ZGC: Add support for JFR leak profiler

[Stefan Karlsson]

Hi Erik,

This looks good to me!

I think it would be good for the code to limit the number of 
UnifedOopRef::addr calls, since those are unsafe ways into the the addr 
and _could_ be abused to go around the UnifiedOopRef encapsulation. 
Maybe something for the future.

Just a small nit:

https://cr.openjdk.java.net/~eosterlund/8235174/webrev.00/src/hotspot/share/jfr/leakprofiler/checkpoint/eventEmitter.cpp.frames.html

Now that you've limited the use of object_addr, maybe you could also 
limit its scope by moving

113   const oop* object_addr = sample->object_addr();

to around:

123 edge = edge_store->put(UnifiedOopRef::encode_in_native(object_addr));


Thanks,
StefanK

On 2019-12-02 11:09, erik.osterlund at oracle.com wrote:
> Hi,
>
> The JFR leak profiler is currently not supported on ZGC, because it 
> has not been accessorized appropriately.
> This patch adds the required Access API sprinkling necessary, to 
> enable this for ZGC. I did the following:
>
> 1) Renamed UnifiedOop to UnifiedOopRef, because it describes the 
> reference to an oop, not the oop itself.
> 2) Changed UnifiedOopRef to be a POD, instead of an AllStatic that 
> passes around its encoded data as
>    const oop* (even though it might be a tagged pointer to a narrow 
> oop), improving type safty.
> 3) Added another tag bit to UnifiedOopRef, allowing it to keep track 
> of whether the described location is
>    IN_NATIVE or IN_HEAP. The dereference function performs the 
> appropriate NativeAccess/HeapAccess with the
>    AS_NO_KEEPALIVE decotator, which is fine because the leak profiler 
> does not create any edges to oops
>    found during traversal.
> 4) Removed code blob oop iteration, because it can't be supported 
> appropriately by the current UnifiedOopRef
>    mechanism (because oops are misaligned, and UnifiedOopRef requires 
> tag bits - I believe this has never
>    worked properly)
> 6) Accessorized loads on strong roots to 
> NativeAccess<AS_NO_KEEPALIVE>::oop_load. JFR leak profiler never
>    creates new references to oops that are visited, so it does not 
> need to keep things alive
> 7) Explicitly told the oop iteration closures to not visit referents. 
> The default referent strategy used
>    before is DO_DISCOVERY, which doesn't seem right.
> 8) Fixed a pre-existing issue where the array index reported in the 
> leak chain was calculated incorrectly
>    as the byte offset from the array base. It should be scaled by the 
> element length of the array.
>
> Ran some JFR leak profiler tests, and some ad-hoc leak profiling in 
> various applications. Also checked that
> there were no observable perf regressions in the iterations, and there 
> were not.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8235174
>
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8235174/webrev.00/
>
> Thanks,
> /Erik