Low-Overhead Heap Profiling
robbin.ehn at oracle.com
Thu Jan 25 09:15:27 UTC 2018
Hi JC, great to see another revision!
StackTraceData should not contain the oop for 'safety' reasons.
When StackTraceData is moved from _allocated_traces:
it contains a dead oop.
_allocated_traces could instead be a tupel of oop and StackTraceData thus dead
oops are not kept.
You should use the new Access API for loading the oop, something like this:
RootAccess<ON_PHANTOM_OOP_REF | AS_NO_KEEPALIVE>::load(...)
I don't think you need to use Access API for clearing the oop, but it would look
nicer. And you shouldn't probably be using: Universe::heap()->is_in_reserved(value)
L424 MutexLocker mu(HeapMonitorStorage_lock);
Is not needed as far as I can see.
weak_oops_do is called in a safepoint, no TLAB allocation can happen and JVMTI
thread can't access these data-structures. Is there something more to this lock
that I'm missing?
You have 6 files without any changes in them (any more):
I have not looked closely, but is it possible to hide heap sampling in
AllocTracer ? (with some minor changes to the AllocTracer API)
Minor nit, when declaring pointer there is a little mix of having the pointer
adjacent by type name and data name. (Most hotspot code is by type name)
heapMonitoring.cpp:711 jvmtiStackTrace *trace = ....
heapMonitoring.cpp:733 Method* m = vfst.method();
(not just this file)
I would make g_tmp volatile, otherwise the assignment in loop may theoretical be
On 01/24/2018 01:40 AM, JC Beyler wrote:
> And it has been exactly two months since I posted an update here :)
> Thanksgiving, Christmas, and handling
> https://bugs.openjdk.java.net/browse/JDK-8190862 will do that to you
> I have gotten back to this item now that JDK-8190862 is done and I
> have the following webrev ready:
> With the incremental here:
> The updates are:
> a) GetCachedTraces
> - I added a GC collection if the GetLiveTraces is called, this is
> because you really want to know what is currently live when you call
> that method
> - If you are worried about performance and only care about what was
> live at the last GC, you can now call GetCachedTraces, which does not
> provoke a GC
> Let me know if there are any questions here or concerns. I'm happy to
> explain and defend the choices :).
> Note: I added a new test for this and updated other tests to double
> check the live call. (see for example
> b) Disabling samples wipes out the data
> I had originally implemented for OpenJdk a version that keeps the data
> in memory after disabling the sampler, allowing a user to get traces
> post-sampling. Because of this, we would always do the weak_oops_do
> method, whether enabled or disabled. This led to a slight regression
> in performance for GC reference processing time. I had initially fixed
> this with a small "was this ever enabled" flag. This would have
> allowed a program that never uses this to not have a regression but a
> program that enables the disabled the code for the rest of the
> execution would still pay the price after disabling the sampler.
> Because of this, I have moved things back to where they probably
> should be: if you disable the sampler, you lose the data. But this
> allows a simpler code path: if the sampler is disabled, skip over the
> GC weak_oops_do method.
> Let me know what you think and happy 2018 to all!
> On Thu, Nov 23, 2017 at 7:20 AM, Thomas Schatzl
> <thomas.schatzl at oracle.com> wrote:
>> On Tue, 2017-11-21 at 13:50 -0800, JC Beyler wrote:
>>> Hi all,
>>> I have a new webrev here:
>>> With the incremental one here:
>>> I think I did most items from Thomans and Robbin. I've especially
>>> added a new test:
>> Thanks. Looks good.
More information about the hotspot-compiler-dev