Skip to content

Commit

Permalink
8300915: G1: incomplete SATB because nmethod entry barriers don't get…
Browse files Browse the repository at this point in the history
… armed

Reviewed-by: tschatzl, eosterlund
  • Loading branch information
reinrich committed Jan 30, 2023
1 parent cbefe1f commit 3db558b
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 15 deletions.
3 changes: 3 additions & 0 deletions src/hotspot/share/code/codeCache.cpp
Expand Up @@ -863,6 +863,9 @@ void CodeCache::on_gc_marking_cycle_start() {
++_gc_epoch;
}

// Once started the code cache marking cycle must only be finished after marking of
// the java heap is complete. Otherwise nmethods could appear to be not on stack even
// if they have frames in continuation StackChunks that were not yet visited.
void CodeCache::on_gc_marking_cycle_finish() {
assert(is_gc_marking_cycle_active(), "Marking cycle started before last one finished");
++_gc_epoch;
Expand Down
22 changes: 15 additions & 7 deletions src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Expand Up @@ -3360,15 +3360,23 @@ void G1CollectedHeap::fill_with_dummy_object(HeapWord* start, HeapWord* end, boo
region->fill_with_dummy_object(start, pointer_delta(end, start), zap);
}

void G1CollectedHeap::start_codecache_marking_cycle_if_inactive() {
void G1CollectedHeap::start_codecache_marking_cycle_if_inactive(bool concurrent_mark_start) {
// We can reach here with an active code cache marking cycle either because the
// previous G1 concurrent marking cycle was undone (if heap occupancy after the
// concurrent start young collection was below the threshold) or aborted. See
// CodeCache::on_gc_marking_cycle_finish() why this is. We must not start a new code
// cache cycle then. If we are about to start a new g1 concurrent marking cycle we
// still have to arm all nmethod entry barriers. They are needed for adding oop
// constants to the SATB snapshot. Full GC does not need nmethods to be armed.
if (!CodeCache::is_gc_marking_cycle_active()) {
// This is the normal case when we do not call collect when a
// concurrent mark is ongoing. We then start a new code marking
// cycle. If, on the other hand, a concurrent mark is ongoing, we
// will be conservative and use the last code marking cycle. Code
// caches marked between the two concurrent marks will live a bit
// longer than needed.
CodeCache::on_gc_marking_cycle_start();
}
if (concurrent_mark_start) {
CodeCache::arm_all_nmethods();
}
}

void G1CollectedHeap::finish_codecache_marking_cycle() {
CodeCache::on_gc_marking_cycle_finish();
CodeCache::arm_all_nmethods();
}
3 changes: 2 additions & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.hpp
Expand Up @@ -933,7 +933,8 @@ class G1CollectedHeap : public CollectedHeap {

void fill_with_dummy_object(HeapWord* start, HeapWord* end, bool zap) override;

static void start_codecache_marking_cycle_if_inactive();
static void start_codecache_marking_cycle_if_inactive(bool concurrent_mark_start);
static void finish_codecache_marking_cycle();

// Apply the given closure on all cards in the Hot Card Cache, emptying it.
void iterate_hcc_closure(G1CardTableEntryClosure* cl, uint worker_id);
Expand Down
7 changes: 3 additions & 4 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
Expand Up @@ -792,7 +792,7 @@ G1PreConcurrentStartTask::G1PreConcurrentStartTask(GCCause::Cause cause, G1Concu
void G1ConcurrentMark::pre_concurrent_start(GCCause::Cause cause) {
assert_at_safepoint_on_vm_thread();

G1CollectedHeap::start_codecache_marking_cycle_if_inactive();
G1CollectedHeap::start_codecache_marking_cycle_if_inactive(true /* concurrent_mark_start */);

ClassLoaderDataGraph::verify_claimed_marks_cleared(ClassLoaderData::_claim_strong);

Expand Down Expand Up @@ -1300,6 +1300,8 @@ void G1ConcurrentMark::remark() {
assert(!restart_for_overflow(), "sanity");
// Completely reset the marking state (except bitmaps) since marking completed.
reset_at_marking_complete();

G1CollectedHeap::finish_codecache_marking_cycle();
} else {
// We overflowed. Restart concurrent marking.
_restart_for_overflow = true;
Expand All @@ -1316,9 +1318,6 @@ void G1ConcurrentMark::remark() {
report_object_count(mark_finished);
}

CodeCache::on_gc_marking_cycle_finish();
CodeCache::arm_all_nmethods();

// Statistics
double now = os::elapsedTime();
_remark_mark_times.add((mark_work_end - start) * 1000.0);
Expand Down
5 changes: 2 additions & 3 deletions src/hotspot/share/gc/g1/g1FullCollector.cpp
Expand Up @@ -197,7 +197,7 @@ void G1FullCollector::prepare_collection() {
}

void G1FullCollector::collect() {
G1CollectedHeap::start_codecache_marking_cycle_if_inactive();
G1CollectedHeap::start_codecache_marking_cycle_if_inactive(false /* concurrent_mark_start */);

phase1_mark_live_objects();
verify_after_marking();
Expand All @@ -211,8 +211,7 @@ void G1FullCollector::collect() {

phase4_do_compaction();

CodeCache::on_gc_marking_cycle_finish();
CodeCache::arm_all_nmethods();
G1CollectedHeap::finish_codecache_marking_cycle();
}

void G1FullCollector::complete_collection() {
Expand Down

1 comment on commit 3db558b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.