Skip to content

Commit

Permalink
8274264: Not all of G1 young collection verification honors the verif…
Browse files Browse the repository at this point in the history
…ication type

Reviewed-by: ayang, tschatzl
  • Loading branch information
ahmedmuhsin authored and Thomas Schatzl committed Mar 9, 2023
1 parent a7e308a commit 34a9246
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 15 deletions.
20 changes: 16 additions & 4 deletions src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Expand Up @@ -970,8 +970,11 @@ void G1CollectedHeap::verify_before_full_collection(bool explicit_gc) {
if (!VerifyBeforeGC) {
return;
}
if (!G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyFull)) {
return;
}
_verifier->verify_region_sets_optional();
_verifier->verify_before_gc(G1HeapVerifier::G1VerifyFull);
_verifier->verify_before_gc();
_verifier->verify_bitmap_clear(true /* above_tams_only */);
}

Expand Down Expand Up @@ -1009,9 +1012,12 @@ void G1CollectedHeap::verify_after_full_collection() {
if (!VerifyAfterGC) {
return;
}
if (!G1HeapVerifier::should_verify(G1HeapVerifier::G1VerifyFull)) {
return;
}
_hrm.verify_optional();
_verifier->verify_region_sets_optional();
_verifier->verify_after_gc(G1HeapVerifier::G1VerifyFull);
_verifier->verify_after_gc();
_verifier->verify_bitmap_clear(false /* above_tams_only */);

// At this point there should be no regions in the
Expand Down Expand Up @@ -2573,11 +2579,14 @@ void G1CollectedHeap::verify_before_young_collection(G1HeapVerifier::G1VerifyTyp
if (!VerifyBeforeGC) {
return;
}
if (!G1HeapVerifier::should_verify(type)) {
return;
}
Ticks start = Ticks::now();
_verifier->prepare_for_verify();
_verifier->verify_region_sets_optional();
_verifier->verify_dirty_young_regions();
_verifier->verify_before_gc(type);
_verifier->verify_before_gc();
verify_numa_regions("GC Start");
phase_times()->record_verify_before_time_ms((Ticks::now() - start).seconds() * MILLIUNITS);
}
Expand All @@ -2586,8 +2595,11 @@ void G1CollectedHeap::verify_after_young_collection(G1HeapVerifier::G1VerifyType
if (!VerifyAfterGC) {
return;
}
if (!G1HeapVerifier::should_verify(type)) {
return;
}
Ticks start = Ticks::now();
_verifier->verify_after_gc(type);
_verifier->verify_after_gc();
verify_numa_regions("GC End");
_verifier->verify_region_sets_optional();
phase_times()->record_verify_after_time_ms((Ticks::now() - start).seconds() * MILLIUNITS);
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
Expand Up @@ -1071,14 +1071,14 @@ void G1ConcurrentMark::verify_during_pause(G1HeapVerifier::G1VerifyType type,

const char* caller = verify_location_string(location);

if (VerifyDuringGC) {
if (VerifyDuringGC && G1HeapVerifier::should_verify(type)) {
GCTraceTime(Debug, gc, phases) debug(caller, _gc_timer_cm);

size_t const BufLen = 512;
char buffer[BufLen];

jio_snprintf(buffer, BufLen, "During GC (%s)", caller);
verifier->verify(type, VerifyOption::G1UseConcMarking, buffer);
verifier->verify(VerifyOption::G1UseConcMarking, buffer);

// Only check bitmap in Remark, and not at After-Verification because the regions
// already have their TAMS'es reset.
Expand Down
12 changes: 6 additions & 6 deletions src/hotspot/share/gc/g1/g1HeapVerifier.cpp
Expand Up @@ -551,19 +551,19 @@ void G1HeapVerifier::prepare_for_verify() {
}
}

void G1HeapVerifier::verify(G1VerifyType type, VerifyOption vo, const char* msg) {
if (should_verify(type) && _g1h->total_collections() >= VerifyGCStartAt) {
void G1HeapVerifier::verify(VerifyOption vo, const char* msg) {
if (_g1h->total_collections() >= VerifyGCStartAt) {
prepare_for_verify();
Universe::verify(vo, msg);
}
}

void G1HeapVerifier::verify_before_gc(G1VerifyType type) {
verify(type, VerifyOption::G1UseConcMarking, "Before GC");
void G1HeapVerifier::verify_before_gc() {
verify(VerifyOption::G1UseConcMarking, "Before GC");
}

void G1HeapVerifier::verify_after_gc(G1VerifyType type) {
verify(type, VerifyOption::G1UseConcMarking, "After GC");
void G1HeapVerifier::verify_after_gc() {
verify(VerifyOption::G1UseConcMarking, "After GC");
}

void G1HeapVerifier::verify_bitmap_clear(bool from_tams) {
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/gc/g1/g1HeapVerifier.hpp
Expand Up @@ -66,9 +66,9 @@ class G1HeapVerifier : public CHeapObj<mtGC> {
void verify_region_sets_optional() { DEBUG_ONLY(verify_region_sets();) }

void prepare_for_verify();
void verify(G1VerifyType type, VerifyOption vo, const char* msg);
void verify_before_gc(G1VerifyType type);
void verify_after_gc(G1VerifyType type);
void verify(VerifyOption vo, const char* msg);
void verify_before_gc();
void verify_after_gc();

void verify_bitmap_clear(bool above_tams_only);

Expand Down

1 comment on commit 34a9246

@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.