[11u] RFR 8255269: Unsigned overflow in g1Policy.cpp

[Kemper, William]

G1GCPhaseTimes::OptObjCopy was added in 13 during a larger refactoring effort (https://bugs.openjdk.java.net/browse/JDK-8218668). This enum value doesn't exist in 11 so it's expected that it's not used in the patch for 11.

Thanks,
William
________________________________________
From: John Cuthbertson <johnc at azul.com>
Sent: Wednesday, November 4, 2020 8:51 AM
To: Hohensee, Paul
Cc: Kemper, William; jdk-updates-dev at openjdk.java.net
Subject: RE: [EXTERNAL] [11u] RFR 8255269: Unsigned overflow in g1Policy.cpp

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



Hi William, Paul,

Does this patch have the same issue as before — the missed 'average_time_ms(G1GCPhaseTimes::OptObjCopy)’ term in the calculation of average_copy_time? Will that be resolved by back porting Paul’s fix?

JohnC

> On Nov 4, 2020, at 8:18 AM, Hohensee, Paul <hohensee at amazon.com> wrote:
>
> Thanks, lgtm now. I'll tag the issue.
>
> Paul
>
> On 11/3/20, 5:23 PM, "jdk-updates-dev on behalf of Kemper, William" <jdk-updates-dev-retn at openjdk.java.net on behalf of kemperw at amazon.com> wrote:
>
>    Here's a patch based on jdk13u using the Skara CLI tools:
>
>    git hg-export 7e95c0a5
>    # HG changeset patch
>    # User phh
>    # Date 1603811625 0
>    #      Tue Oct 15:13:45 2020 +0000
>    8255269: Unsigned overflow in g1Policy.cpp
>    Reviewed-by: yan
>    Contributed-by: William Kemper <kemperw at amazon.com<mailto:kemperw at amazon.com>>
>
>    diff --git a/src/hotspot/share/gc/g1/g1Policy.cpp b/src/hotspot/share/gc/g1/g1Policy.cpp
>    index 91eceb7375..dec7f57de6 100644
>    --- a/src/hotspot/share/gc/g1/g1Policy.cpp
>    +++ b/src/hotspot/share/gc/g1/g1Policy.cpp
>    @@ -1,5 +1,5 @@
>     /*
>    - * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
>    + * Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
>      * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>      *
>      * This code is free software; you can redistribute it and/or modify it
>    @@ -692,11 +692,11 @@ void G1Policy::record_collection_pause_end(double pause_time_ms, size_t cards_sc
>         _analytics->report_rs_length_diff((double) rs_length_diff);
>
>         size_t freed_bytes = heap_used_bytes_before_gc - cur_used_bytes;
>    -    size_t copied_bytes = _collection_set->bytes_used_before() - freed_bytes;
>    -    double cost_per_byte_ms = 0.0;
>
>    -    if (copied_bytes > 0) {
>    -      cost_per_byte_ms = (average_time_ms(G1GCPhaseTimes::ObjCopy) + average_time_ms(G1GCPhaseTimes::OptObjCopy)) / (double) copied_bytes;
>    +    if (_collection_set->bytes_used_before() > freed_bytes) {
>    +      size_t copied_bytes = _collection_set->bytes_used_before() - freed_bytes;
>    +      double average_copy_time = average_time_ms(G1GCPhaseTimes::ObjCopy);
>    +      double cost_per_byte_ms = average_copy_time / (double) copied_bytes;
>           _analytics->report_cost_per_byte_ms(cost_per_byte_ms, collector_state()->mark_or_rebuild_in_progress());
>         }
>
>    I ran the jtreg tests for G1 against the release build on my workstation (x86, Ubuntu 18.04). The results were the same before and after the patch was applied:
>
>    test/hotspot/jtreg/gc/g1
>    Test results: passed: 56; failed: 4; error: 1
>
>    It doesn't look like the failures and error have anything to do with this change.
>
>    Thanks,
>    William
>