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

[John Cuthbertson]

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
>