RFR: 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale [v4]

[Afshin Zafari]

On Thu, 12 Jan 2023 16:11:18 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 74:
> 
>> 72: 
>> 73:   // Convert diff amount in bytes to current reporting scale
>> 74:   int64_t diff_in_current_scale(size_t s1, size_t s2) const {
> 
> - Can you add a little comment please about why you use int64_t? E.g. something like:
> 
> // We use int64_t instead of ssize_t because on 32-bit it allows us to express deltas larger than 2 gb. On 64-bit we
> // never expect memory sizes larger than INT64_MAX".
> 
> - I would assert the input values before doing the subtraction, then don't need the is_negative part. You could shrink the function to something like this:
>  ```
>   int64_t diff_in_current_scale(size_t s1, size_t s2) const {
>     LP64_ONLY(assert(s1 < INT64_MAX && s2 < INT64_MAX, "size overflow");)
>     return ((int64_t)s1 - (int64_t)s2)/scale;
>  }
> 
> The LP64_ONLY could be needed because on 32-bit the compiler may complain about the compare being always true.
> 
> Am I overlooking something?

The difference should be round up/down to an exact number, i.e.  +/- scale/2.
s1 and s2 can be both very large, even larger than INT64_MAX, but the difference and round off is our concern here in this function. As an 8bit example: `s1 = 200` and `s2 = 220`  both larger than 127 = INT8_MAX.

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

PR: https://git.openjdk.org/jdk/pull/11514