[8u] RFR (S) 8260704: ParallelGC: oldgen expansion needs release-store for _end

[Hohensee, Paul]

Lgtm. I've created an openjdk8u backport issue https://bugs.openjdk.java.net/browse/JDK-8262794 (new'ish process), which you can tag.

Thanks,
Paul

-----Original Message-----
From: jdk8u-dev <jdk8u-dev-retn at openjdk.java.net> on behalf of Aleksey Shipilev <shade at redhat.com>
Date: Monday, March 1, 2021 at 10:15 AM
To: "jdk8u-dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
Subject: [8u] RFR (S) 8260704: ParallelGC: oldgen expansion needs release-store for _end

This is third of three patches that dance around the same issue.

Original change:
   https://bugs.openjdk.java.net/browse/JDK-8260704
   https://github.com/openjdk/jdk16/commit/afd5eefd

Applies on top of JDK-8257999 (effectively reverting everything but the extended comment) and
JDK-8259271 (which brings orderAccess.hpp include to mutableSpace.cpp).

Unfortunately, 8u code does not have OrderAccess cleanups, so we have to use the older forms of
release_store. 8u variant is:

iff -r 9b1a140fca5a src/share/vm/gc_implementation/parallelScavenge/psOldGen.cpp
--- a/src/share/vm/gc_implementation/parallelScavenge/psOldGen.cpp      Fri Jan 22 11:20:52 2021 +0000
+++ b/src/share/vm/gc_implementation/parallelScavenge/psOldGen.cpp      Mon Mar 01 19:10:34 2021 +0100
@@ -1,4 +1,4 @@
  /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
@@ -33,5 +33,4 @@
  #include "oops/oop.inline.hpp"
  #include "runtime/java.hpp"
-#include "runtime/orderAccess.hpp"

  PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC
@@ -403,7 +402,7 @@
    Universe::heap()->barrier_set()->resize_covered_region(new_memregion);

-  // Ensure the space bounds are updated and made visible to other
-  // threads after the other data structures have been resized.
-  OrderAccess::storestore();
+  // The update of the space's end is done by this call.  As that
+  // makes the new space available for concurrent allocation, this
+  // must be the last step when expanding.
    object_space()->initialize(new_memregion,
                               SpaceDecorator::DontClear,
diff -r 9b1a140fca5a src/share/vm/gc_implementation/shared/mutableSpace.cpp
--- a/src/share/vm/gc_implementation/shared/mutableSpace.cpp    Fri Jan 22 11:20:52 2021 +0000
+++ b/src/share/vm/gc_implementation/shared/mutableSpace.cpp    Mon Mar 01 19:10:34 2021 +0100
@@ -1,4 +1,4 @@
  /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
@@ -126,5 +126,9 @@

    set_bottom(mr.start());
-  set_end(mr.end());
+  // When expanding concurrently with callers of cas_allocate, setting end
+  // makes the new space available for allocation by other threads.  So this
+  // assignment must follow all other configuration and initialization that
+  // might be done for expansion.
+  OrderAccess::release_store_ptr(end_addr(), mr.end());

    if (clear_space) {

Testing: tier1

--
Thanks,
-Aleksey