From efc392259c64986bbbe880259e95b09058b9076a Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Tue, 28 Nov 2023 09:49:03 +0000 Subject: [PATCH] 8319048: Monitor deflation unlink phase prolongs time to safepoint Reviewed-by: ysr, stefank, aboldtch, dcubed --- src/hotspot/share/runtime/globals.hpp | 4 + src/hotspot/share/runtime/synchronizer.cpp | 66 +++++-- src/hotspot/share/runtime/synchronizer.hpp | 1 + .../Monitor/MonitorUnlinkBatchTest.java | 178 ++++++++++++++++++ 4 files changed, 235 insertions(+), 14 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/Monitor/MonitorUnlinkBatchTest.java diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index e28017a0d4b..7a5b8615594 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -736,6 +736,10 @@ const int ObjectAlignmentInBytes = 8; "at one time (minimum is 1024).") \ range(1024, max_jint) \ \ + product(intx, MonitorUnlinkBatch, 500, DIAGNOSTIC, \ + "The maximum number of monitors to unlink in one batch. ") \ + range(1, max_jint) \ + \ product(int, MonitorUsedDeflationThreshold, 90, DIAGNOSTIC, \ "Percentage of used monitors before triggering deflation (0 is " \ "off). The check is performed on GuaranteedSafepointInterval, " \ diff --git a/src/hotspot/share/runtime/synchronizer.cpp b/src/hotspot/share/runtime/synchronizer.cpp index 28bfbb6d71c..4d01037380d 100644 --- a/src/hotspot/share/runtime/synchronizer.cpp +++ b/src/hotspot/share/runtime/synchronizer.cpp @@ -84,44 +84,68 @@ size_t MonitorList::max() const { return Atomic::load(&_max); } -// Walk the in-use list and unlink (at most MonitorDeflationMax) deflated -// ObjectMonitors. Returns the number of unlinked ObjectMonitors. +// Walk the in-use list and unlink deflated ObjectMonitors. +// Returns the number of unlinked ObjectMonitors. size_t MonitorList::unlink_deflated(Thread* current, LogStream* ls, elapsedTimer* timer_p, + size_t deflated_count, GrowableArray* unlinked_list) { size_t unlinked_count = 0; ObjectMonitor* prev = nullptr; - ObjectMonitor* head = Atomic::load_acquire(&_head); - ObjectMonitor* m = head; + ObjectMonitor* m = Atomic::load_acquire(&_head); + // The in-use list head can be null during the final audit. while (m != nullptr) { if (m->is_being_async_deflated()) { - // Find next live ObjectMonitor. + // Find next live ObjectMonitor. Batch up the unlinkable monitors, so we can + // modify the list once per batch. The batch starts at "m". + size_t unlinked_batch = 0; ObjectMonitor* next = m; + // Look for at most MonitorUnlinkBatch monitors, or the number of + // deflated and not unlinked monitors, whatever comes first. + assert(deflated_count >= unlinked_count, "Sanity: underflow"); + size_t unlinked_batch_limit = MIN2(deflated_count - unlinked_count, MonitorUnlinkBatch); do { ObjectMonitor* next_next = next->next_om(); - unlinked_count++; + unlinked_batch++; unlinked_list->append(next); next = next_next; - if (unlinked_count >= (size_t)MonitorDeflationMax) { - // Reached the max so bail out on the gathering loop. + if (unlinked_batch >= unlinked_batch_limit) { + // Reached the max batch, so bail out of the gathering loop. + break; + } + if (prev == nullptr && Atomic::load(&_head) != m) { + // Current batch used to be at head, but it is not at head anymore. + // Bail out and figure out where we currently are. This avoids long + // walks searching for new prev during unlink under heavy list inserts. break; } } while (next != nullptr && next->is_being_async_deflated()); + + // Unlink the found batch. if (prev == nullptr) { - ObjectMonitor* prev_head = Atomic::cmpxchg(&_head, head, next); - if (prev_head != head) { - // Find new prev ObjectMonitor that just got inserted. + // The current batch is the first batch, so there is a chance that it starts at head. + // Optimistically assume no inserts happened, and try to unlink the entire batch from the head. + ObjectMonitor* prev_head = Atomic::cmpxchg(&_head, m, next); + if (prev_head != m) { + // Something must have updated the head. Figure out the actual prev for this batch. for (ObjectMonitor* n = prev_head; n != m; n = n->next_om()) { prev = n; } + assert(prev != nullptr, "Should have found the prev for the current batch"); prev->set_next_om(next); } } else { + // The current batch is preceded by another batch. This guarantees the current batch + // does not start at head. Unlink the entire current batch without updating the head. + assert(Atomic::load(&_head) != m, "Sanity"); prev->set_next_om(next); } - if (unlinked_count >= (size_t)MonitorDeflationMax) { - // Reached the max so bail out on the searching loop. + + unlinked_count += unlinked_batch; + if (unlinked_count >= deflated_count) { + // Reached the max so bail out of the searching loop. + // There should be no more deflated monitors left. break; } m = next; @@ -137,6 +161,20 @@ size_t MonitorList::unlink_deflated(Thread* current, LogStream* ls, ls, timer_p); } } + +#ifdef ASSERT + // Invariant: the code above should unlink all deflated monitors. + // The code that runs after this unlinking does not expect deflated monitors. + // Notably, attempting to deflate the already deflated monitor would break. + { + ObjectMonitor* m = Atomic::load_acquire(&_head); + while (m != nullptr) { + assert(!m->is_being_async_deflated(), "All deflated monitors should be unlinked"); + m = m->next_om(); + } + } +#endif + Atomic::sub(&_count, unlinked_count); return unlinked_count; } @@ -1625,7 +1663,7 @@ size_t ObjectSynchronizer::deflate_idle_monitors() { // Unlink deflated ObjectMonitors from the in-use list. ResourceMark rm; GrowableArray delete_list((int)deflated_count); - unlinked_count = _in_use_list.unlink_deflated(current, ls, &timer, &delete_list); + unlinked_count = _in_use_list.unlink_deflated(current, ls, &timer, deflated_count, &delete_list); if (current->is_monitor_deflation_thread()) { if (ls != nullptr) { timer.stop(); diff --git a/src/hotspot/share/runtime/synchronizer.hpp b/src/hotspot/share/runtime/synchronizer.hpp index d65a1e14bfa..4dea13432e7 100644 --- a/src/hotspot/share/runtime/synchronizer.hpp +++ b/src/hotspot/share/runtime/synchronizer.hpp @@ -47,6 +47,7 @@ private: public: void add(ObjectMonitor* monitor); size_t unlink_deflated(Thread* current, LogStream* ls, elapsedTimer* timer_p, + size_t deflated_count, GrowableArray* unlinked_list); size_t count() const; size_t max() const; diff --git a/test/hotspot/jtreg/runtime/Monitor/MonitorUnlinkBatchTest.java b/test/hotspot/jtreg/runtime/Monitor/MonitorUnlinkBatchTest.java new file mode 100644 index 00000000000..063940ce643 --- /dev/null +++ b/test/hotspot/jtreg/runtime/Monitor/MonitorUnlinkBatchTest.java @@ -0,0 +1,178 @@ +/* + * Copyright Amazon.com Inc. 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/* + * @test id=defaults + * @bug 8319048 + * @summary Test the MonitorUnlinkBatch options + * @library /test/lib + * @run driver MonitorUnlinkBatchTest defaults + */ + +/* + * @test id=legal + * @library /test/lib + * @run driver MonitorUnlinkBatchTest legal + */ + +/* + * @test id=illegal + * @library /test/lib + * @run driver MonitorUnlinkBatchTest illegal + */ + +/* + * @test id=aggressive + * @library /test/lib + * @run driver MonitorUnlinkBatchTest aggressive + */ + +/* + * @test id=lazy + * @library /test/lib + * @run driver MonitorUnlinkBatchTest lazy + */ + + +public class MonitorUnlinkBatchTest { + + public static class Test { + // Inflate a lot of monitors, so that threshold heuristics definitely fires + private static final int MONITORS = 10_000; + + // Use a handful of threads to inflate the monitors, to eat the cost of + // wait(1) calls. This can be larger than available parallelism, since threads + // would be time-waiting. + private static final int THREADS = 16; + + private static Thread[] threads; + private static Object[] monitors; + + public static void main(String... args) throws Exception { + monitors = new Object[MONITORS]; + threads = new Thread[THREADS]; + + for (int t = 0; t < THREADS; t++) { + int monStart = t * MONITORS / THREADS; + int monEnd = (t + 1) * MONITORS / THREADS; + threads[t] = new Thread(() -> { + for (int m = monStart; m < monEnd; m++) { + Object o = new Object(); + synchronized (o) { + try { + o.wait(1); + } catch (InterruptedException e) { + } + } + monitors[m] = o; + } + }); + threads[t].start(); + } + + for (Thread t : threads) { + t.join(); + } + + try { + Thread.sleep(10_000); + } catch (InterruptedException ie) { + } + } + } + + public static void main(String[] args) throws Exception { + if (args.length < 1) { + throw new IllegalArgumentException("Expect the test label"); + } + + String test = args[0]; + switch (test) { + case "defaults": + test(""); + break; + + case "legal": + // Legal, even if not useful settings + test("", + "-XX:MonitorDeflationMax=100000", + "-XX:MonitorUnlinkBatch=100001" + ); + break; + + case "illegal": + // Quick tests that should fail on JVM flags verification. + test("outside the allowed range", + "-XX:MonitorUnlinkBatch=-1" + ); + test("outside the allowed range", + "-XX:MonitorUnlinkBatch=0" + ); + break; + + case "aggressive": + // The smallest batch possible. + test("", + "-XX:MonitorUnlinkBatch=1" + ); + break; + + case "lazy": + // The largest batch possible. + test("", + "-XX:MonitorDeflationMax=1000000", + "-XX:MonitorUnlinkBatch=1000000" + ); + break; + + default: + throw new IllegalArgumentException("Unknown test: " + test); + } + } + + public static void test(String msg, String... args) throws Exception { + List opts = new ArrayList<>(); + opts.add("-Xmx128M"); + opts.add("-XX:+UnlockDiagnosticVMOptions"); + opts.add("-XX:GuaranteedAsyncDeflationInterval=100"); + opts.addAll(Arrays.asList(args)); + opts.add("MonitorUnlinkBatchTest$Test"); + + ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder(opts); + OutputAnalyzer oa = new OutputAnalyzer(pb.start()); + if (msg.isEmpty()) { + oa.shouldHaveExitValue(0); + } else { + oa.shouldNotHaveExitValue(0); + oa.shouldContain(msg); + } + } + +}