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