Re: RFR: 8261447: MethodInvocationCounters frequently run into overflow [v7]

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261447: MethodInvocationCounters frequently run into overflow [v7]

Lutz Schmidt
> 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

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2511/files
  - new: https://git.openjdk.java.net/jdk/pull/2511/files/faab64b0..0f220ee3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2511&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2511&range=05-06

  Stats: 8 lines in 6 files changed: 0 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2511.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2511/head:pull/2511

PR: https://git.openjdk.java.net/jdk/pull/2511
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261447: MethodInvocationCounters frequently run into overflow [v7]

Lutz Schmidt
On Wed, 24 Feb 2021 22:28:15 GMT, Martin Doerr <[hidden email]> wrote:

>> Lutz Schmidt has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   update copyright year
>
> 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).

I agree. The comments could be misleading. They are gone.

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

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