Skip to content

Commit

Permalink
8322503: Shenandoah: Clarify gc state usage
Browse files Browse the repository at this point in the history 
Reviewed-by: ysr, gli
  • Loading branch information
William Kemper authored and Y. Srinivas Ramakrishna committed Jan 2, 2024
1 parent 9481d06 commit 51238c4
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 18 deletions.
14 changes: 7 additions & 7 deletions src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,7 @@ void ShenandoahHeap::prepare_update_heap_references(bool concurrent) {
_update_refs_iterator.reset();
}

void ShenandoahHeap::set_gc_state_all_threads() {
void ShenandoahHeap::propagate_gc_state_to_java_threads() {
assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint");
if (_gc_state_changed) {
_gc_state_changed = false;
Expand All @@ -1753,21 +1753,21 @@ void ShenandoahHeap::set_gc_state_all_threads() {
}
}

void ShenandoahHeap::set_gc_state_mask(uint mask, bool value) {
void ShenandoahHeap::set_gc_state(uint mask, bool value) {
assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at Shenandoah safepoint");
_gc_state.set_cond(mask, value);
_gc_state_changed = true;
}

void ShenandoahHeap::set_concurrent_mark_in_progress(bool in_progress) {
assert(!has_forwarded_objects(), "Not expected before/after mark phase");
set_gc_state_mask(MARKING, in_progress);
set_gc_state(MARKING, in_progress);
ShenandoahBarrierSet::satb_mark_queue_set().set_active_all_threads(in_progress, !in_progress);
}

void ShenandoahHeap::set_evacuation_in_progress(bool in_progress) {
assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Only call this at safepoint");
set_gc_state_mask(EVACUATION, in_progress);
set_gc_state(EVACUATION, in_progress);
}

void ShenandoahHeap::set_concurrent_strong_root_in_progress(bool in_progress) {
Expand All @@ -1779,7 +1779,7 @@ void ShenandoahHeap::set_concurrent_strong_root_in_progress(bool in_progress) {
}

void ShenandoahHeap::set_concurrent_weak_root_in_progress(bool cond) {
set_gc_state_mask(WEAK_ROOTS, cond);
set_gc_state(WEAK_ROOTS, cond);
}

GCTracer* ShenandoahHeap::tracer() {
Expand Down Expand Up @@ -1906,7 +1906,7 @@ void ShenandoahHeap::parallel_cleaning(bool full_gc) {
}

void ShenandoahHeap::set_has_forwarded_objects(bool cond) {
set_gc_state_mask(HAS_FORWARDED, cond);
set_gc_state(HAS_FORWARDED, cond);
}

void ShenandoahHeap::set_unload_classes(bool uc) {
Expand Down Expand Up @@ -1945,7 +1945,7 @@ void ShenandoahHeap::set_full_gc_move_in_progress(bool in_progress) {
}

void ShenandoahHeap::set_update_refs_in_progress(bool in_progress) {
set_gc_state_mask(UPDATEREFS, in_progress);
set_gc_state(UPDATEREFS, in_progress);
}

void ShenandoahHeap::register_nmethod(nmethod* nm) {
Expand Down
13 changes: 10 additions & 3 deletions src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,19 @@ class ShenandoahHeap : public CollectedHeap, public ShenandoahSpaceInfo {

size_t _gc_no_progress_count;

void set_gc_state_mask(uint mask, bool value);
// This updates the singlular, global gc state. This must happen on a safepoint.
void set_gc_state(uint mask, bool value);

public:
char gc_state() const;
void set_gc_state_all_threads();
bool has_gc_state_changed() { return _gc_state_changed; }

// This copies the global gc state into a thread local variable for java threads.
// It is primarily intended to support quick access at barriers.
void propagate_gc_state_to_java_threads();

// This is public to support assertions that the state hasn't been changed off of
// a safepoint and that any changes were propagated to java threads after the safepoint.
bool has_gc_state_changed() const { return _gc_state_changed; }

void set_concurrent_mark_in_progress(bool in_progress);
void set_evacuation_in_progress(bool in_progress);
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/gc/shenandoah/shenandoahThreadLocalData.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class ShenandoahThreadLocalData {
}

static char gc_state(Thread* thread) {
assert(thread->is_Java_thread(), "GC state is only synchronized to java threads");
return data(thread)->_gc_state;
}

Expand Down
14 changes: 7 additions & 7 deletions src/hotspot/share/gc/shenandoah/shenandoahVMOperations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,41 +62,41 @@ void VM_ShenandoahReferenceOperation::doit_epilogue() {
void VM_ShenandoahInitMark::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Init Mark", SvcGCMarker::CONCURRENT);
_gc->entry_init_mark();
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}

void VM_ShenandoahFinalMarkStartEvac::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Final Mark", SvcGCMarker::CONCURRENT);
_gc->entry_final_mark();
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}

void VM_ShenandoahFullGC::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Full GC", SvcGCMarker::FULL);
_full_gc->entry_full(_gc_cause);
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}

void VM_ShenandoahDegeneratedGC::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Degenerated GC", SvcGCMarker::CONCURRENT);
_gc->entry_degenerated();
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}

void VM_ShenandoahInitUpdateRefs::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Init Update Refs", SvcGCMarker::CONCURRENT);
_gc->entry_init_updaterefs();
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}

void VM_ShenandoahFinalUpdateRefs::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Final Update Refs", SvcGCMarker::CONCURRENT);
_gc->entry_final_updaterefs();
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}

void VM_ShenandoahFinalRoots::doit() {
ShenandoahGCPauseMark mark(_gc_id, "Final Roots", SvcGCMarker::CONCURRENT);
_gc->entry_final_roots();
ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();
}
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ void ShenandoahVerifier::verify_at_safepoint(const char *label,
guarantee(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "only when nothing else happens");
guarantee(ShenandoahVerify, "only when enabled, and bitmap is initialized in ShenandoahHeap::initialize");

ShenandoahHeap::heap()->set_gc_state_all_threads();
ShenandoahHeap::heap()->propagate_gc_state_to_java_threads();

// Avoid side-effect of changing workers' active thread count, but bypass concurrent/parallel protocol check
ShenandoahPushWorkerScope verify_worker_scope(_heap->workers(), _heap->max_workers(), false /*bypass check*/);
Expand Down

7 comments on commit 51238c4

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

@earthling-amzn
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport jdk21u-dev

@earthling-amzn
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@earthling-amzn
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport jdk22u

@openjdk
Copy link

@openjdk openjdk bot commented on 51238c4 Mar 13, 2024

Choose a reason for hiding this comment

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

@earthling-amzn Could not automatically backport 51238c4b to openjdk/jdk21u-dev due to conflicts in the following files:

  • src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk21u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk21u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b backport-earthling-amzn-51238c4b

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b

# Backport the commit
$ git cherry-pick --no-commit 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk21u-dev with the title Backport 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 51238c4b from the openjdk/jdk repository.

The commit being backported was authored by William Kemper on 2 Jan 2024 and was reviewed by Y. Srinivas Ramakrishna and Guoxiong Li.

Thanks!

@openjdk
Copy link

@openjdk openjdk bot commented on 51238c4 Mar 13, 2024

Choose a reason for hiding this comment

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

@earthling-amzn the backport was successfully created on the branch backport-earthling-amzn-51238c4b in my personal fork of openjdk/jdk22u. To create a pull request with this backport targeting openjdk/jdk22u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 51238c4b from the openjdk/jdk repository.

The commit being backported was authored by William Kemper on 2 Jan 2024 and was reviewed by Y. Srinivas Ramakrishna and Guoxiong Li.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk22u:

$ git fetch https://github.com/openjdk-bots/jdk22u.git backport-earthling-amzn-51238c4b:backport-earthling-amzn-51238c4b
$ git checkout backport-earthling-amzn-51238c4b
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk22u.git backport-earthling-amzn-51238c4b

⚠️ @earthling-amzn You are not yet a collaborator in my fork openjdk-bots/jdk22u. An invite will be sent out and you need to accept it before you can proceed.

@openjdk
Copy link

@openjdk openjdk bot commented on 51238c4 Mar 13, 2024

Choose a reason for hiding this comment

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

@earthling-amzn Could not automatically backport 51238c4b to openjdk/jdk17u-dev due to conflicts in the following files:

  • src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b backport-earthling-amzn-51238c4b

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b

# Backport the commit
$ git cherry-pick --no-commit 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 51238c4bdbce48f6b719f7dcfe5b7df8b8c6d85b.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 51238c4b from the openjdk/jdk repository.

The commit being backported was authored by William Kemper on 2 Jan 2024 and was reviewed by Y. Srinivas Ramakrishna and Guoxiong Li.

Thanks!

Please sign in to comment.