On Fri, 16 May 2025 18:43:35 GMT, Serguei Spitsyn <sspit...@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+kstef...@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
