RFR: 8261447: MethodInvocationCounters frequently run into overflow [v8]

[Igor Veresov]

On Thu, 25 Feb 2021 16:36:38 GMT, Igor Veresov <iveresov at openjdk.org> wrote:

>> Lutz Schmidt has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   comment changes requested by TheRealMDoerr
>
> src/hotspot/share/runtime/java.cpp line 100:
> 
>> 98: int compare_methods(Method** a, Method** b) {
>> 99:   return (int32_t)(((uint32_t)(*b)->invocation_count() + (*b)->compiled_invocation_count())
>> 100:                  - ((uint32_t)(*a)->invocation_count() + (*a)->compiled_invocation_count()));
> 
> Is this correct? The arithmetic look to be: (int32_t) (uint64_t - uint64_t). If the 64 values inside don't fit in 32, you'll get a negative number which would break the sorting logic.

I see that you've fixed the types since the last comment, but it think it's still broken (and has been before).
How about:
int64_t diff =  ((*b)->compiled_invocation_count() - (*a)->compiled_invocation_count()) + ((*b)->invocation_count() - (*a)->invocation_count());
if (diff > 0) return 1;
else if (diff < 0) return -1;
else return 0;
It's kind of hacky too, because it assumes that compiled_invocation_count() are positive and didn't overflow. But at least we'd get rid of a possible overflow during summation. What do you think?

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

PR: https://git.openjdk.java.net/jdk/pull/2511