RFR: 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale [v4]
Afshin Zafari
duke at openjdk.org
Fri Jan 13 08:26:23 UTC 2023
On Thu, 12 Jan 2023 16:31:42 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>>
>> - 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>> - Merge branch 'master' into _8281213
>> - 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>> - 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>> - 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>> - 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>> - 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>
> src/hotspot/share/services/memReporter.hpp line 88:
>
>> 86: size_t scaled = (amount / _scale);
>> 87: if ((amount % _scale) > (_scale - 1)/2) {
>> 88: scaled += 1;
>
> This correction is not needed I think.
>
> The old code did not do this, so it may cause NMT numbers to seemingly go up after this patch, suggesting we use more memory. And it's wrong either way.
>
> A better way to deal with this (as a future improvement) would be to print 2-3 significant digits, e.g. "2.33GB". Like we do in the `proper_unit_for_byte_size()` and friends, but with a fixed scale.
In the tests, there are cases where explicit numbers are expected as the result of the difference function, like +256K or -4K. So, if the tests pass with this code, it means than all the results are as NMT expected.
-------------
PR: https://git.openjdk.org/jdk/pull/11514
More information about the hotspot-runtime-dev
mailing list