8376335

Hi all,

  please review this change to make the `PreservedMarks` classes use `Atomic<T>`.

Some refactoring allowed removal of a now unnecessary function, and imo improved the code, avoiding the somewhat awkward passing of the atomic variable.

Testing: gha

Thanks,
  Thomas

src/hotspot/share/gc/shared/preservedMarks.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2026, 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
@@ -29,15 +29,17 @@
 #include "memory/allocation.inline.hpp"
 #include "memory/resourceArea.hpp"
 #include "oops/oop.inline.hpp"
-#include "runtime/atomicAccess.hpp"
+#include "runtime/atomic.hpp"
 #include "utilities/macros.hpp"
 
-void PreservedMarks::restore() {
+size_t PreservedMarks::restore() {
+  size_t result = size();
   while (!_stack.is_empty()) {
     const PreservedMark elem = _stack.pop();
     elem.set_mark();
   }
   assert_empty();
+  return result;
 }
 
 void PreservedMarks::adjust_preserved_mark(PreservedMark* elem) {
@@ -55,15 +57,6 @@ void PreservedMarks::adjust_during_full_gc() {
   }
 }
 
-void PreservedMarks::restore_and_increment(volatile size_t* const total_size_addr) {
-  const size_t stack_size = size();
-  restore();
-  // Only do the atomic add if the size is > 0.
-  if (stack_size > 0) {
-    AtomicAccess::add(total_size_addr, stack_size);
-  }
-}
-
 #ifndef PRODUCT
 void PreservedMarks::assert_empty() {
   assert(_stack.is_empty(), "stack expected to be empty, size = %zu",
@@ -93,7 +86,7 @@ void PreservedMarksSet::init(uint num) {
 class RestorePreservedMarksTask : public WorkerTask {
   PreservedMarksSet* const _preserved_marks_set;
   SequentialSubTasksDone _sub_tasks;
-  volatile size_t _total_size;
+  Atomic<size_t> _total_size;
 #ifdef ASSERT
   size_t _total_size_before;
 #endif // ASSERT
@@ -102,7 +95,10 @@ public:
   void work(uint worker_id) override {
     uint task_id = 0;
     while (_sub_tasks.try_claim_task(task_id)) {
-      _preserved_marks_set->get(task_id)->restore_and_increment(&_total_size);
+      size_t num_restored = _preserved_marks_set->get(task_id)->restore();
+      if (num_restored > 0) {
+        _total_size.add_then_fetch(num_restored);
+      }
     }
   }
 
@@ -121,9 +117,11 @@ public:
   }
 
   ~RestorePreservedMarksTask() {
-    assert(_total_size == _total_size_before, "total_size = %zu before = %zu", _total_size, _total_size_before);
-    size_t mem_size = _total_size * (sizeof(oop) + sizeof(markWord));
-    log_trace(gc)("Restored %zu marks, occupying %zu %s", _total_size,
+    size_t local_total_size = _total_size.load_relaxed();
+
+    assert(local_total_size == _total_size_before, "total_size = %zu before = %zu", local_total_size, _total_size_before);
+    size_t mem_size = local_total_size * (sizeof(oop) + sizeof(markWord));
+    log_trace(gc)("Restored %zu marks, occupying %zu %s", local_total_size,
                                                           byte_size_in_proper_unit(mem_size),
                                                           proper_unit_for_byte_size(mem_size));
   }

src/hotspot/share/gc/shared/preservedMarks.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2026, 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
@@ -61,7 +61,8 @@ public:
   inline void push_always(oop obj, markWord m);
   // Iterate over the stack, restore all preserved marks, and
   // reclaim the memory taken up by the stack segments.
-  void restore();
+  // Returns the number of marks restored.
+  size_t restore();
 
   // Adjust the preserved mark according to its
   // forwarding location stored in the mark.
@@ -71,8 +72,6 @@ public:
   // to their forwarding location stored in the mark.
   void adjust_during_full_gc();
 
-  void restore_and_increment(volatile size_t* const _total_size_addr);
-
   // Assert the stack is empty and has no cached segments.
   void assert_empty() PRODUCT_RETURN;