On Thu, 25 Feb 2021 16:36:38 GMT, Igor Veresov <[email protected]> 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
