Skip to content

8369178: G1: Use NMethodMarkingScope and ThreadsClaimTokenScope in G1RootProcessor #27644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+14 −46
Open

8369178: G1: Use NMethodMarkingScope and ThreadsClaimTokenScope in G1RootProcessor #27644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e73adde
replace
fandreuz Oct 6, 2025
131f636
revert
fandreuz Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
#include "gc/g1/g1RemSet.hpp"
#include "gc/g1/g1ReviseYoungLengthTask.hpp"
#include "gc/g1/g1RootClosures.hpp"
#include "gc/g1/g1RootProcessor.hpp"
#include "gc/g1/g1SATBMarkQueueSet.hpp"
#include "gc/g1/g1ServiceThread.hpp"
#include "gc/g1/g1ThreadLocalData.hpp"
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1HeapVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ void G1HeapVerifier::verify(VerifyOption vo) {
G1VerifyCodeRootNMethodClosure blobsCl(&codeRootsCl);

{
G1RootProcessor root_processor(_g1h, 1);
G1RootProcessor root_processor(_g1h, false /* is_parallel */);
root_processor.process_all_roots(&rootsCl, &cldCl, &blobsCl);
}

Expand Down
13 changes: 5 additions & 8 deletions src/hotspot/share/gc/g1/g1RootProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@
#include "utilities/enumIterator.hpp"
#include "utilities/macros.hpp"

G1RootProcessor::G1RootProcessor(G1CollectedHeap* g1h, uint n_workers) :
G1RootProcessor::G1RootProcessor(G1CollectedHeap* g1h, bool is_parallel) :
_g1h(g1h),
_process_strong_tasks(G1RP_PS_NumElements),
_srs(n_workers) {}
_nmethod_marking_scope(),
_threads_claim_token_scope(),
_is_parallel(is_parallel) {}

void G1RootProcessor::evacuate_roots(G1ParScanThreadState* pss, uint worker_id) {
G1GCPhaseTimes* phase_times = _g1h->phase_times();
Expand Down Expand Up @@ -175,8 +177,7 @@ void G1RootProcessor::process_java_roots(G1RootClosures* closures,
// oops_do_process_weak and oops_do_process_strong in nmethod.hpp
{
G1GCParPhaseTimesTracker x(phase_times, G1GCPhaseTimes::ThreadRoots, worker_id);
bool is_par = n_workers() > 1;
Threads::possibly_parallel_oops_do(is_par,
Threads::possibly_parallel_oops_do(_is_parallel,
closures->strong_oops(),
closures->strong_nmethods());
}
Expand Down Expand Up @@ -209,7 +210,3 @@ void G1RootProcessor::process_code_cache_roots(NMethodClosure* nmethod_closure,
CodeCache::nmethods_do(nmethod_closure);
}
}

uint G1RootProcessor::n_workers() const {
return _srs.n_threads();
}
12 changes: 6 additions & 6 deletions src/hotspot/share/gc/g1/g1RootProcessor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
#ifndef SHARE_GC_G1_G1ROOTPROCESSOR_HPP
#define SHARE_GC_G1_G1ROOTPROCESSOR_HPP

#include "code/nmethod.hpp"
#include "gc/shared/oopStorageSetParState.hpp"
#include "gc/shared/strongRootsScope.hpp"
#include "memory/allocation.hpp"
#include "runtime/mutex.hpp"
#include "runtime/threads.hpp"

class CLDClosure;
class G1CollectedHeap;
Expand All @@ -48,7 +49,9 @@ class SubTasksDone;
class G1RootProcessor : public StackObj {
G1CollectedHeap* _g1h;
SubTasksDone _process_strong_tasks;
StrongRootsScope _srs;
NMethodMarkingScope _nmethod_marking_scope;
ThreadsClaimTokenScope _threads_claim_token_scope;
bool _is_parallel;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have used uint _num_workers, but I guess this also works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to know how many workers there are, just if we are parallel or not. I think this goes in the same direction as the idea of the PR, being clear about what we actually need

OopStorageSetStrongParState<false, false> _oop_storage_set_strong_par_state;

enum G1H_process_roots_tasks {
Expand All @@ -72,7 +75,7 @@ class G1RootProcessor : public StackObj {
uint worker_id);

public:
G1RootProcessor(G1CollectedHeap* g1h, uint n_workers);
G1RootProcessor(G1CollectedHeap* g1h, bool is_parallel);

// Apply correct closures from pss to the strongly and weakly reachable roots in the system
// in a single pass.
Expand All @@ -88,9 +91,6 @@ class G1RootProcessor : public StackObj {
void process_all_roots(OopClosure* oops,
CLDClosure* clds,
NMethodClosure* nmethods);

// Number of worker threads used by the root processor.
uint n_workers() const;
};

#endif // SHARE_GC_G1_G1ROOTPROCESSOR_HPP
3 changes: 2 additions & 1 deletion src/hotspot/share/gc/g1/g1YoungCollector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "gc/shared/gcTimer.hpp"
#include "gc/shared/gcTraceTime.inline.hpp"
#include "gc/shared/referenceProcessor.hpp"
#include "gc/shared/strongRootsScope.hpp"
#include "gc/shared/weakProcessor.inline.hpp"
#include "gc/shared/workerPolicy.hpp"
#include "gc/shared/workerThread.hpp"
Expand Down Expand Up @@ -750,7 +751,7 @@ void G1YoungCollector::evacuate_initial_collection_set(G1ParScanThreadStateSet*

Ticks start_processing = Ticks::now();
{
G1RootProcessor root_processor(_g1h, num_workers);
G1RootProcessor root_processor(_g1h, num_workers > 1 /* is_parallel */);
G1EvacuateRegionsTask g1_par_task(_g1h,
per_thread_states,
task_queues(),
Expand Down
15 changes: 0 additions & 15 deletions src/hotspot/share/gc/shared/strongRootsScope.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,3 @@ MarkScope::MarkScope() {
MarkScope::~MarkScope() {
nmethod::oops_do_marking_epilogue();
}

StrongRootsScope::StrongRootsScope(uint n_threads) : _n_threads(n_threads) {
// No need for thread claim for statically-known sequential case (_n_threads == 0)
// For positive values, clients of this class often unify sequential/parallel
// cases, so they expect the thread claim token to be updated.
if (_n_threads != 0) {
Threads::change_thread_claim_token();
}
}

StrongRootsScope::~StrongRootsScope() {
if (_n_threads != 0) {
Threads::assert_all_threads_claimed();
}
}
13 changes: 0 additions & 13 deletions src/hotspot/share/gc/shared/strongRootsScope.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,4 @@ class MarkScope : public StackObj {
~MarkScope();
};

// Sets up and tears down the required state for sequential/parallel root processing.
class StrongRootsScope : public MarkScope {
// Number of threads participating in the roots processing.
// 0 means statically-known sequential root processing; used only by Serial GC
const uint _n_threads;

public:
StrongRootsScope(uint n_threads);
~StrongRootsScope();

uint n_threads() const { return _n_threads; }
};

#endif // SHARE_GC_SHARED_STRONGROOTSSCOPE_HPP
1 change: 0 additions & 1 deletion src/hotspot/share/runtime/safepoint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include "gc/shared/collectedHeap.hpp"
#include "gc/shared/gcLocker.hpp"
#include "gc/shared/oopStorage.hpp"
#include "gc/shared/strongRootsScope.hpp"
#include "gc/shared/workerThread.hpp"
#include "gc/shared/workerUtils.hpp"
#include "interpreter/interpreter.hpp"
Expand Down