From 96a542feb2064dba155ebf05311752995d164038 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Tue, 19 Jul 2022 16:32:07 +0000 Subject: [PATCH] 8227060: Optimize safepoint cleanup subtask order Reviewed-by: kbarrett, pchilanomate --- .../share/compiler/compilationPolicy.hpp | 3 +- src/hotspot/share/runtime/safepoint.cpp | 57 ++++++++----------- src/hotspot/share/runtime/safepoint.hpp | 1 - src/hotspot/share/runtime/synchronizer.cpp | 2 +- .../runtime/logging/SafepointCleanupTest.java | 3 +- 5 files changed, 26 insertions(+), 40 deletions(-) diff --git a/src/hotspot/share/compiler/compilationPolicy.hpp b/src/hotspot/share/compiler/compilationPolicy.hpp index 3526348d55495..2a5745f7c149a 100644 --- a/src/hotspot/share/compiler/compilationPolicy.hpp +++ b/src/hotspot/share/compiler/compilationPolicy.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2022, 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 @@ -252,7 +252,6 @@ class CompilationPolicy : AllStatic { static bool can_be_osr_compiled(const methodHandle& m, int comp_level = CompLevel_any); static bool is_compilation_enabled(); - static void do_safepoint_work() { } static CompileTask* select_task_helper(CompileQueue* compile_queue); // Return initial compile level to use with Xcomp (depends on compilation mode). static void reprofile(ScopeDesc* trap_scope, bool is_osr); diff --git a/src/hotspot/share/runtime/safepoint.cpp b/src/hotspot/share/runtime/safepoint.cpp index 67d8e19b60ddc..eb459798b504e 100644 --- a/src/hotspot/share/runtime/safepoint.cpp +++ b/src/hotspot/share/runtime/safepoint.cpp @@ -516,19 +516,9 @@ bool SafepointSynchronize::is_cleanup_needed() { return false; } -class ParallelSPCleanupThreadClosure : public ThreadClosure { -public: - void do_thread(Thread* thread) { - if (thread->is_Java_thread()) { - StackWatermarkSet::start_processing(JavaThread::cast(thread), StackWatermarkKind::gc); - } - } -}; - -class ParallelSPCleanupTask : public WorkerTask { +class ParallelCleanupTask : public WorkerTask { private: SubTasksDone _subtasks; - uint _num_workers; bool _do_lazy_roots; class Tracer { @@ -548,32 +538,14 @@ class ParallelSPCleanupTask : public WorkerTask { }; public: - ParallelSPCleanupTask(uint num_workers) : + ParallelCleanupTask() : WorkerTask("Parallel Safepoint Cleanup"), _subtasks(SafepointSynchronize::SAFEPOINT_CLEANUP_NUM_TASKS), - _num_workers(num_workers), _do_lazy_roots(!VMThread::vm_operation()->skip_thread_oop_barriers() && Universe::heap()->uses_stack_watermark_barrier()) {} void work(uint worker_id) { - if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_LAZY_ROOT_PROCESSING)) { - if (_do_lazy_roots) { - Tracer t("lazy partial thread root processing"); - ParallelSPCleanupThreadClosure cl; - Threads::threads_do(&cl); - } - } - - if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_UPDATE_INLINE_CACHES)) { - Tracer t("updating inline caches"); - InlineCacheBuffer::update_inline_caches(); - } - - if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_COMPILATION_POLICY)) { - Tracer t("compilation policy safepoint handler"); - CompilationPolicy::do_safepoint_work(); - } - + // These tasks are ordered by relative length of time to execute so that potentially longer tasks start first. if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_SYMBOL_TABLE_REHASH)) { if (SymbolTable::needs_rehashing()) { Tracer t("rehashing symbol table"); @@ -595,6 +567,25 @@ class ParallelSPCleanupTask : public WorkerTask { } } + if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_LAZY_ROOT_PROCESSING)) { + if (_do_lazy_roots) { + Tracer t("lazy partial thread root processing"); + class LazyRootClosure : public ThreadClosure { + public: + void do_thread(Thread* thread) { + StackWatermarkSet::start_processing(JavaThread::cast(thread), StackWatermarkKind::gc); + } + }; + LazyRootClosure cl; + Threads::java_threads_do(&cl); + } + } + + if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_UPDATE_INLINE_CACHES)) { + Tracer t("updating inline caches"); + InlineCacheBuffer::update_inline_caches(); + } + if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_REQUEST_OOPSTORAGE_CLEANUP)) { // Don't bother reporting event or time for this very short operation. // To have any utility we'd also want to report whether needed. @@ -612,15 +603,13 @@ void SafepointSynchronize::do_cleanup_tasks() { CollectedHeap* heap = Universe::heap(); assert(heap != NULL, "heap not initialized yet?"); + ParallelCleanupTask cleanup; WorkerThreads* cleanup_workers = heap->safepoint_workers(); if (cleanup_workers != NULL) { // Parallel cleanup using GC provided thread pool. - uint num_cleanup_workers = cleanup_workers->active_workers(); - ParallelSPCleanupTask cleanup(num_cleanup_workers); cleanup_workers->run_task(&cleanup); } else { // Serial cleanup using VMThread. - ParallelSPCleanupTask cleanup(1); cleanup.work(0); } diff --git a/src/hotspot/share/runtime/safepoint.hpp b/src/hotspot/share/runtime/safepoint.hpp index 7c9ff959dbe87..f323252074244 100644 --- a/src/hotspot/share/runtime/safepoint.hpp +++ b/src/hotspot/share/runtime/safepoint.hpp @@ -72,7 +72,6 @@ class SafepointSynchronize : AllStatic { enum SafepointCleanupTasks { SAFEPOINT_CLEANUP_LAZY_ROOT_PROCESSING, SAFEPOINT_CLEANUP_UPDATE_INLINE_CACHES, - SAFEPOINT_CLEANUP_COMPILATION_POLICY, SAFEPOINT_CLEANUP_SYMBOL_TABLE_REHASH, SAFEPOINT_CLEANUP_STRING_TABLE_REHASH, SAFEPOINT_CLEANUP_SYSTEM_DICTIONARY_RESIZE, diff --git a/src/hotspot/share/runtime/synchronizer.cpp b/src/hotspot/share/runtime/synchronizer.cpp index aad3d092ac377..51250014cf524 100644 --- a/src/hotspot/share/runtime/synchronizer.cpp +++ b/src/hotspot/share/runtime/synchronizer.cpp @@ -1671,7 +1671,7 @@ void ObjectSynchronizer::do_final_audit_and_print_stats() { ; // empty } // The other audit_and_print_stats() call is done at the Debug - // level at a safepoint in ObjectSynchronizer::do_safepoint_work(). + // level at a safepoint in SafepointSynchronize::do_cleanup_tasks. ObjectSynchronizer::audit_and_print_stats(true /* on_exit */); } } diff --git a/test/hotspot/jtreg/runtime/logging/SafepointCleanupTest.java b/test/hotspot/jtreg/runtime/logging/SafepointCleanupTest.java index b7d2407c18e02..82859b38f15a9 100644 --- a/test/hotspot/jtreg/runtime/logging/SafepointCleanupTest.java +++ b/test/hotspot/jtreg/runtime/logging/SafepointCleanupTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2022, 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 @@ -41,7 +41,6 @@ static void analyzeOutputOn(ProcessBuilder pb) throws Exception { output.shouldContain("[safepoint,cleanup]"); output.shouldContain("safepoint cleanup tasks"); output.shouldContain("updating inline caches"); - output.shouldContain("compilation policy safepoint handler"); output.shouldHaveExitValue(0); }