On Wed, 17 Feb 2021 18:22:01 GMT, Lutz Schmidt <l...@openjdk.org> wrote:
>> Dear community,
>> may I please request reviews for this fix, improving the usefulness of
>> method invocation counters.
>> - aggregation counters are retyped as uint64_t, shifting the overflow
>> probability way out (185 days in case of a 1 GHz counter update frequency).
>> - counters for individual methods are interpreted as (unsigned int), in
>> contrast to their declaration as int. This gives us a factor of two before
>> the counters overflow.
>> - as a special case, "compiled_invocation_counter" is retyped as long,
>> because it has a higher update frequency than other counters.
>> - before/after sample output is attached to the bug description.
>>
>> Thank you!
>> Lutz
>
> Lutz Schmidt has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update copyright year
This version looks ok. I understand that you don't want to clean up the whole
singed / unsigned mess. That's fine with me. I'd only like to see one confusing
comment removed or replaced.
You may also want to check your 64 bit overflow time in the description: I
guess 185 days matches a 1 THz counter update frequency. With 1 GHz it should
be above 500 years.
src/hotspot/share/runtime/java.cpp line 100:
> 98: int compare_methods(Method** a, Method** b) {
> 99: // invocation_count() may have overflowed already. Interpret it's
> result as
> 100: // unsigned int to shift the limit of meaningless results by a factor
> of 2.
Code is fine, but this comment doesn't make sense to me. The result is the same
with your version. But it has the advantage that it avoids signed integer
overflow (undefined behavior).
-------------
Changes requested by mdoerr (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2511
