JDK-8171119: Low-Overhead Heap Profiling

JC Beyler jcbeyler at google.com
Tue Jan 30 03:22:57 UTC 2018


Hi Robbin,

So I did the changes to move most of the code into the AllocTracer and
you can see it incrementally here:
http://cr.openjdk.java.net/~jcbeyler/8171119/webrev.04_05/
And the full webrev here:
http://cr.openjdk.java.net/~jcbeyler/8171119/webrev.05/

Now the issues I see here:

- AllocTracer now does "things" instead of just having a send_*_event
API, that is a change in concept for that class
- Collectedheap still needs to call AllocTracer to see if it is to be
sampled, I can't hide everything in it without a bigger refactor (want
me to try?)
- The ordering is now important: AllocTracer has to get called before
tlab().fill
   - Otherwise the fact that the allocation has to get sampled will
get lost when the tlab gets replaced

If this still looks better to you or in a better direction, let me
know. I can do the end part of it and add an event for a sample since
now that is easy and makes sense to probably add such an event, and I
can add a few more tests.

Thanks!

On Mon, Jan 29, 2018 at 1:29 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
> Hi JC, thanks!
>
> I'm happy with current state, looks good!
>
> Truncated:
>
> On 01/27/2018 05:01 AM, JC Beyler wrote:
>>
>> This is strange but I'm assuming it is because we are not working on
>> the same repo?
>>
>> I used:
>> hg clone http://hg.openjdk.java.net/jdk/hs jdkhs-heap
>>
>> I'll try a new clone on Monday and see. My new version moved hard_end
>> back to public so it should work now.
>
>
> Sorry this compile error was in closed code.
> Now the closed part compiles, thanks.
>
>>
>> Fair enough, hopefully Thomas will chime in. Are you saying that this
>> first version could go in and we can work on a refinement? Or are you
>> saying I should work on this now at the same time and fix it before
>> this V1 goes in? (Just so I know :))
>
>
> We may have to change this before integration, but for now keep it as is.
>
>> I'll look at this on Monday then!
>
>
> Great!
>
> /Robbin
>
>
>>
>> Thanks for the reply and have a great weekend!
>> Jc
>>
>>>
>>>>
>>>>> ####
>>>>> 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)
>>>>> E.g.
>>>>> heapMonitoring.cpp:711     jvmtiStackTrace *trace = ....
>>>>> heapMonitoring.cpp:733         Method* m = vfst.method();
>>>>> (not just this file)
>>>>>
>>>>
>>>> Done!
>>>>
>>>>> ####
>>>>> HeapMonitorThreadOnOffTest.java:77
>>>>> I would make g_tmp volatile, otherwise the assignment in loop may
>>>>> theoretical be skipped.
>>>>>
>>>>
>>>> Also done!
>>>
>>>
>>>
>>> Looks good, thanks!
>>>
>>> /Robbin
>>>
>>>>
>>>> Thanks again!
>>>> Jc
>>>>
>>>
>


More information about the serviceability-dev mailing list