RFR: 8356372: JVMTI heap sampling not working properly with outside TLAB allocations [v3]

[Stefan Karlsson]

On Fri, 16 May 2025 18:43:35 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Stefan Karlsson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Apply suggestions from code review
>>   
>>   Co-authored-by: Stefan Johansson <54407259+kstefanj at users.noreply.github.com>
>
> src/hotspot/share/runtime/threadHeapSampler.cpp line 399:
> 
>> 397:   assert(result > 0 && result < static_cast<double>(SIZE_MAX), "Result is not in an acceptable range.");
>> 398:   size_t interval = static_cast<size_t>(result);
>> 399:   _sample_threshold = interval;
> 
> Nit: The line 399 is not needed as the value is reset at 400.

Nice catch. This is debugging code from the early stage of this patch. It set up a consistent state to make it easier to get consistent data from the test runs. This also means that we need to respin our testing to make sure that the randomization doesn't trigger anything that we didn't catch with the fixed _sample_threshold.

> src/hotspot/share/runtime/threadHeapSampler.hpp line 82:
> 
>> 80:       _tlab_top_at_sample_start(nullptr),
>> 81:       _accumulated_tlab_bytes_since_sample(0),
>> 82:       _accumulated_outside_tlab_bytes_since_sample(0) {
> 
> Nit: Would it make sense to initialize `_sample_threshold` as well to make it consistent?

To me it makes sense. I didn't do it here because it gets initialized further down and I didn't want reviewers to get hung up on that change. But I'll take this as a go-ahead to make that change.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25114#discussion_r2094996935
PR Review Comment: https://git.openjdk.org/jdk/pull/25114#discussion_r2095001254