8267188: gc/stringdedup/TestStringDeduplicationInterned.java fails with Shenandoah

Reviewed-by: rkennke, shade
This commit is contained in:
Zhengyu Gu 2021-08-25 20:16:25 +00:00
parent e36cbd8e05
commit 7212561dd1
8 changed files with 84 additions and 77 deletions

View File

@ -86,9 +86,11 @@ public:
ShenandoahObjToScanQueue* q = _cm->get_queue(worker_id);
ShenandoahReferenceProcessor* rp = heap->ref_processor();
assert(rp != NULL, "need reference processor");
StringDedup::Requests requests;
_cm->mark_loop(worker_id, _terminator, rp,
true /*cancellable*/,
ShenandoahStringDedup::is_enabled() ? ENQUEUE_DEDUP : NO_DEDUP);
ShenandoahStringDedup::is_enabled() ? ENQUEUE_DEDUP : NO_DEDUP,
&requests);
}
};
@ -134,6 +136,7 @@ public:
ShenandoahParallelWorkerSession worker_session(worker_id);
ShenandoahReferenceProcessor* rp = heap->ref_processor();
StringDedup::Requests requests;
// First drain remaining SATB buffers.
{
@ -144,14 +147,15 @@ public:
while (satb_mq_set.apply_closure_to_completed_buffer(&cl)) {}
assert(!heap->has_forwarded_objects(), "Not expected");
ShenandoahMarkRefsClosure<NO_DEDUP> mark_cl(q, rp);
ShenandoahMarkRefsClosure mark_cl(q, rp);
ShenandoahSATBAndRemarkThreadsClosure tc(satb_mq_set,
ShenandoahIUBarrier ? &mark_cl : NULL);
Threads::threads_do(&tc);
}
_cm->mark_loop(worker_id, _terminator, rp,
false /*not cancellable*/,
_dedup_string ? ENQUEUE_DEDUP : NO_DEDUP);
_dedup_string ? ENQUEUE_DEDUP : NO_DEDUP,
&requests);
assert(_cm->task_queues()->is_empty(), "Should be empty");
}
};
@ -189,9 +193,7 @@ ShenandoahMarkConcurrentRootsTask::ShenandoahMarkConcurrentRootsTask(ShenandoahO
void ShenandoahMarkConcurrentRootsTask::work(uint worker_id) {
ShenandoahConcurrentWorkerSession worker_session(worker_id);
ShenandoahObjToScanQueue* q = _queue_set->queue(worker_id);
// Cannot enable string deduplication during root scanning. Otherwise,
// may result lock inversion between stack watermark and string dedup queue lock.
ShenandoahMarkRefsClosure<NO_DEDUP> cl(q, _rp);
ShenandoahMarkRefsClosure cl(q, _rp);
_root_scanner.roots_do(&cl, worker_id);
}

View File

@ -36,7 +36,6 @@
ShenandoahMarkRefsSuperClosure::ShenandoahMarkRefsSuperClosure(ShenandoahObjToScanQueue* q, ShenandoahReferenceProcessor* rp) :
MetadataVisitingOopIterateClosure(rp),
_stringDedup_requests(),
_queue(q),
_mark_context(ShenandoahHeap::heap()->marking_context()),
_weak(false)
@ -56,7 +55,7 @@ void ShenandoahMark::clear() {
}
template <bool CANCELLABLE, StringDedupMode STRING_DEDUP>
void ShenandoahMark::mark_loop_prework(uint w, TaskTerminator *t, ShenandoahReferenceProcessor *rp) {
void ShenandoahMark::mark_loop_prework(uint w, TaskTerminator *t, ShenandoahReferenceProcessor *rp, StringDedup::Requests* const req) {
ShenandoahObjToScanQueue* q = get_queue(w);
ShenandoahHeap* const heap = ShenandoahHeap::heap();
@ -66,23 +65,23 @@ void ShenandoahMark::mark_loop_prework(uint w, TaskTerminator *t, ShenandoahRefe
// play nice with specialized_oop_iterators.
if (heap->unload_classes()) {
if (heap->has_forwarded_objects()) {
using Closure = ShenandoahMarkUpdateRefsMetadataClosure<STRING_DEDUP>;
using Closure = ShenandoahMarkUpdateRefsMetadataClosure;
Closure cl(q, rp);
mark_loop_work<Closure, CANCELLABLE>(&cl, ld, w, t);
mark_loop_work<Closure, CANCELLABLE, STRING_DEDUP>(&cl, ld, w, t, req);
} else {
using Closure = ShenandoahMarkRefsMetadataClosure<STRING_DEDUP>;
using Closure = ShenandoahMarkRefsMetadataClosure;
Closure cl(q, rp);
mark_loop_work<Closure, CANCELLABLE>(&cl, ld, w, t);
mark_loop_work<Closure, CANCELLABLE, STRING_DEDUP>(&cl, ld, w, t, req);
}
} else {
if (heap->has_forwarded_objects()) {
using Closure = ShenandoahMarkUpdateRefsClosure<STRING_DEDUP>;
using Closure = ShenandoahMarkUpdateRefsClosure;
Closure cl(q, rp);
mark_loop_work<Closure, CANCELLABLE>(&cl, ld, w, t);
mark_loop_work<Closure, CANCELLABLE, STRING_DEDUP>(&cl, ld, w, t, req);
} else {
using Closure = ShenandoahMarkRefsClosure<STRING_DEDUP>;
using Closure = ShenandoahMarkRefsClosure;
Closure cl(q, rp);
mark_loop_work<Closure, CANCELLABLE>(&cl, ld, w, t);
mark_loop_work<Closure, CANCELLABLE, STRING_DEDUP>(&cl, ld, w, t, req);
}
}
@ -90,36 +89,36 @@ void ShenandoahMark::mark_loop_prework(uint w, TaskTerminator *t, ShenandoahRefe
}
void ShenandoahMark::mark_loop(uint worker_id, TaskTerminator* terminator, ShenandoahReferenceProcessor *rp,
bool cancellable, StringDedupMode dedup_mode) {
bool cancellable, StringDedupMode dedup_mode, StringDedup::Requests* const req) {
if (cancellable) {
switch(dedup_mode) {
case NO_DEDUP:
mark_loop_prework<true, NO_DEDUP>(worker_id, terminator, rp);
mark_loop_prework<true, NO_DEDUP>(worker_id, terminator, rp, req);
break;
case ENQUEUE_DEDUP:
mark_loop_prework<true, ENQUEUE_DEDUP>(worker_id, terminator, rp);
mark_loop_prework<true, ENQUEUE_DEDUP>(worker_id, terminator, rp, req);
break;
case ALWAYS_DEDUP:
mark_loop_prework<true, ALWAYS_DEDUP>(worker_id, terminator, rp);
mark_loop_prework<true, ALWAYS_DEDUP>(worker_id, terminator, rp, req);
break;
}
} else {
switch(dedup_mode) {
case NO_DEDUP:
mark_loop_prework<false, NO_DEDUP>(worker_id, terminator, rp);
mark_loop_prework<false, NO_DEDUP>(worker_id, terminator, rp, req);
break;
case ENQUEUE_DEDUP:
mark_loop_prework<false, ENQUEUE_DEDUP>(worker_id, terminator, rp);
mark_loop_prework<false, ENQUEUE_DEDUP>(worker_id, terminator, rp, req);
break;
case ALWAYS_DEDUP:
mark_loop_prework<false, ALWAYS_DEDUP>(worker_id, terminator, rp);
mark_loop_prework<false, ALWAYS_DEDUP>(worker_id, terminator, rp, req);
break;
}
}
}
template <class T, bool CANCELLABLE>
void ShenandoahMark::mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint worker_id, TaskTerminator *terminator) {
template <class T, bool CANCELLABLE, StringDedupMode STRING_DEDUP>
void ShenandoahMark::mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint worker_id, TaskTerminator *terminator, StringDedup::Requests* const req) {
uintx stride = ShenandoahMarkLoopStride;
ShenandoahHeap* heap = ShenandoahHeap::heap();
@ -147,7 +146,7 @@ void ShenandoahMark::mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint w
for (uint i = 0; i < stride; i++) {
if (q->pop(t)) {
do_task<T>(q, cl, live_data, &t);
do_task<T, STRING_DEDUP>(q, cl, live_data, req, &t);
} else {
assert(q->is_empty(), "Must be empty");
q = queues->claim_next();
@ -176,7 +175,7 @@ void ShenandoahMark::mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint w
for (uint i = 0; i < stride; i++) {
if (q->pop(t) ||
queues->steal(worker_id, t)) {
do_task<T>(q, cl, live_data, &t);
do_task<T, STRING_DEDUP>(q, cl, live_data, req, &t);
work++;
} else {
break;

View File

@ -45,8 +45,8 @@ protected:
ShenandoahMark();
public:
template<class T, StringDedupMode STRING_DEDUP>
static inline void mark_through_ref(T* p, ShenandoahObjToScanQueue* q, ShenandoahMarkingContext* const mark_context, StringDedup::Requests* const req, bool weak);
template<class T>
static inline void mark_through_ref(T* p, ShenandoahObjToScanQueue* q, ShenandoahMarkingContext* const mark_context, bool weak);
static void clear();
@ -56,8 +56,8 @@ public:
// ---------- Marking loop and tasks
private:
template <class T>
inline void do_task(ShenandoahObjToScanQueue* q, T* cl, ShenandoahLiveData* live_data, ShenandoahMarkTask* task);
template <class T, StringDedupMode STRING_DEDUP>
inline void do_task(ShenandoahObjToScanQueue* q, T* cl, ShenandoahLiveData* live_data, StringDedup::Requests* const req, ShenandoahMarkTask* task);
template <class T>
inline void do_chunked_array_start(ShenandoahObjToScanQueue* q, T* cl, oop array, bool weak);
@ -67,15 +67,17 @@ private:
inline void count_liveness(ShenandoahLiveData* live_data, oop obj);
template <class T, bool CANCELLABLE>
void mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint worker_id, TaskTerminator *t);
template <class T, bool CANCELLABLE,StringDedupMode STRING_DEDUP>
void mark_loop_work(T* cl, ShenandoahLiveData* live_data, uint worker_id, TaskTerminator *t, StringDedup::Requests* const req);
template <bool CANCELLABLE, StringDedupMode STRING_DEDUP>
void mark_loop_prework(uint worker_id, TaskTerminator *terminator, ShenandoahReferenceProcessor *rp);
void mark_loop_prework(uint worker_id, TaskTerminator *terminator, ShenandoahReferenceProcessor *rp, StringDedup::Requests* const req);
template <StringDedupMode STRING_DEDUP>
inline void dedup_string(oop obj, StringDedup::Requests* const req);
protected:
void mark_loop(uint worker_id, TaskTerminator* terminator, ShenandoahReferenceProcessor *rp,
bool cancellable, StringDedupMode dedup_mode);
bool cancellable, StringDedupMode dedup_mode, StringDedup::Requests* const req);
};
#endif // SHARE_GC_SHENANDOAH_SHENANDOAHMARK_HPP

View File

@ -40,8 +40,22 @@
#include "runtime/prefetch.inline.hpp"
#include "utilities/powerOfTwo.hpp"
template <class T>
void ShenandoahMark::do_task(ShenandoahObjToScanQueue* q, T* cl, ShenandoahLiveData* live_data, ShenandoahMarkTask* task) {
template <StringDedupMode STRING_DEDUP>
void ShenandoahMark::dedup_string(oop obj, StringDedup::Requests* const req) {
if (STRING_DEDUP == ENQUEUE_DEDUP) {
if (ShenandoahStringDedup::is_candidate(obj)) {
req->add(obj);
}
} else if (STRING_DEDUP == ALWAYS_DEDUP) {
if (ShenandoahStringDedup::is_string_candidate(obj) &&
!ShenandoahStringDedup::dedup_requested(obj)) {
req->add(obj);
}
}
}
template <class T, StringDedupMode STRING_DEDUP>
void ShenandoahMark::do_task(ShenandoahObjToScanQueue* q, T* cl, ShenandoahLiveData* live_data, StringDedup::Requests* const req, ShenandoahMarkTask* task) {
oop obj = task->obj();
shenandoah_assert_not_forwarded(NULL, obj);
@ -56,6 +70,7 @@ void ShenandoahMark::do_task(ShenandoahObjToScanQueue* q, T* cl, ShenandoahLiveD
if (obj->is_instance()) {
// Case 1: Normal oop, process as usual.
obj->oop_iterate(cl);
dedup_string<STRING_DEDUP>(obj, req);
} else if (obj->is_objArray()) {
// Case 2: Object array instance and no chunk is set. Must be the first
// time we visit it, start the chunked processing.
@ -208,7 +223,6 @@ inline void ShenandoahMark::do_chunked_array(ShenandoahObjToScanQueue* q, T* cl,
class ShenandoahSATBBufferClosure : public SATBBufferClosure {
private:
StringDedup::Requests _stringdedup_requests;
ShenandoahObjToScanQueue* _queue;
ShenandoahHeap* _heap;
ShenandoahMarkingContext* const _mark_context;
@ -222,24 +236,15 @@ public:
void do_buffer(void **buffer, size_t size) {
assert(size == 0 || !_heap->has_forwarded_objects(), "Forwarded objects are not expected here");
if (ShenandoahStringDedup::is_enabled()) {
do_buffer_impl<ENQUEUE_DEDUP>(buffer, size);
} else {
do_buffer_impl<NO_DEDUP>(buffer, size);
}
}
template<StringDedupMode STRING_DEDUP>
void do_buffer_impl(void **buffer, size_t size) {
for (size_t i = 0; i < size; ++i) {
oop *p = (oop *) &buffer[i];
ShenandoahMark::mark_through_ref<oop, STRING_DEDUP>(p, _queue, _mark_context, &_stringdedup_requests, false);
ShenandoahMark::mark_through_ref<oop>(p, _queue, _mark_context, false);
}
}
};
template<class T, StringDedupMode STRING_DEDUP>
inline void ShenandoahMark::mark_through_ref(T* p, ShenandoahObjToScanQueue* q, ShenandoahMarkingContext* const mark_context, StringDedup::Requests* const req, bool weak) {
template<class T>
inline void ShenandoahMark::mark_through_ref(T* p, ShenandoahObjToScanQueue* q, ShenandoahMarkingContext* const mark_context, bool weak) {
T o = RawAccess<>::oop_load(p);
if (!CompressedOops::is_null(o)) {
oop obj = CompressedOops::decode_not_null(o);
@ -257,16 +262,6 @@ inline void ShenandoahMark::mark_through_ref(T* p, ShenandoahObjToScanQueue* q,
if (marked) {
bool pushed = q->push(ShenandoahMarkTask(obj, skip_live, weak));
assert(pushed, "overflow queue should always succeed pushing");
if ((STRING_DEDUP == ENQUEUE_DEDUP) && ShenandoahStringDedup::is_candidate(obj)) {
assert(ShenandoahStringDedup::is_enabled(), "Must be enabled");
req->add(obj);
} else if ((STRING_DEDUP == ALWAYS_DEDUP) &&
ShenandoahStringDedup::is_string_candidate(obj) &&
!ShenandoahStringDedup::dedup_requested(obj)) {
assert(ShenandoahStringDedup::is_enabled(), "Must be enabled");
req->add(obj);
}
}
shenandoah_assert_marked(p, obj);

View File

@ -40,13 +40,12 @@ enum StringDedupMode {
class ShenandoahMarkRefsSuperClosure : public MetadataVisitingOopIterateClosure {
private:
StringDedup::Requests _stringDedup_requests;
ShenandoahObjToScanQueue* _queue;
ShenandoahMarkingContext* const _mark_context;
bool _weak;
protected:
template <class T, StringDedupMode STRING_DEDUP>
template <class T>
void work(T *p);
public:
@ -65,7 +64,7 @@ class ShenandoahMarkUpdateRefsSuperClosure : public ShenandoahMarkRefsSuperClosu
protected:
ShenandoahHeap* const _heap;
template <class T, StringDedupMode STRING_DEDUP>
template <class T>
inline void work(T* p);
public:
@ -76,11 +75,10 @@ public:
};
};
template <StringDedupMode STRING_DEDUP>
class ShenandoahMarkUpdateRefsClosure : public ShenandoahMarkUpdateRefsSuperClosure {
private:
template <class T>
inline void do_oop_work(T* p) { work<T, STRING_DEDUP>(p); }
inline void do_oop_work(T* p) { work<T>(p); }
public:
ShenandoahMarkUpdateRefsClosure(ShenandoahObjToScanQueue* q, ShenandoahReferenceProcessor* rp) :
@ -91,11 +89,10 @@ public:
virtual bool do_metadata() { return false; }
};
template <StringDedupMode STRING_DEDUP>
class ShenandoahMarkUpdateRefsMetadataClosure : public ShenandoahMarkUpdateRefsSuperClosure {
private:
template <class T>
inline void do_oop_work(T* p) { work<T, STRING_DEDUP>(p); }
inline void do_oop_work(T* p) { work<T>(p); }
public:
ShenandoahMarkUpdateRefsMetadataClosure(ShenandoahObjToScanQueue* q, ShenandoahReferenceProcessor* rp) :
@ -107,11 +104,10 @@ public:
};
template <StringDedupMode STRING_DEDUP>
class ShenandoahMarkRefsClosure : public ShenandoahMarkRefsSuperClosure {
private:
template <class T>
inline void do_oop_work(T* p) { work<T, STRING_DEDUP>(p); }
inline void do_oop_work(T* p) { work<T>(p); }
public:
ShenandoahMarkRefsClosure(ShenandoahObjToScanQueue* q, ShenandoahReferenceProcessor* rp) :
@ -123,11 +119,10 @@ public:
};
template <StringDedupMode STRING_DEDUP>
class ShenandoahMarkRefsMetadataClosure : public ShenandoahMarkRefsSuperClosure {
private:
template <class T>
inline void do_oop_work(T* p) { work<T, STRING_DEDUP>(p); }
inline void do_oop_work(T* p) { work<T>(p); }
public:
ShenandoahMarkRefsMetadataClosure(ShenandoahObjToScanQueue* q, ShenandoahReferenceProcessor* rp) :

View File

@ -30,18 +30,18 @@
#include "gc/shenandoah/shenandoahHeap.inline.hpp"
#include "gc/shenandoah/shenandoahMark.inline.hpp"
template<class T, StringDedupMode STRING_DEDUP>
template<class T>
inline void ShenandoahMarkRefsSuperClosure::work(T* p) {
ShenandoahMark::mark_through_ref<T, STRING_DEDUP>(p, _queue, _mark_context, &_stringDedup_requests, _weak);
ShenandoahMark::mark_through_ref<T>(p, _queue, _mark_context, _weak);
}
template<class T, StringDedupMode STRING_DEDUP>
template<class T>
inline void ShenandoahMarkUpdateRefsSuperClosure::work(T* p) {
// Update the location
_heap->update_with_forwarded(p);
// ...then do the usual thing
ShenandoahMarkRefsSuperClosure::work<T, STRING_DEDUP>(p);
ShenandoahMarkRefsSuperClosure::work<T>(p);
}
template<class T>

View File

@ -57,7 +57,7 @@ ShenandoahInitMarkRootsClosure::ShenandoahInitMarkRootsClosure(ShenandoahObjToSc
template <class T>
void ShenandoahInitMarkRootsClosure::do_oop_work(T* p) {
ShenandoahMark::mark_through_ref<T, NO_DEDUP>(p, _queue, _mark_context, NULL, false);
ShenandoahMark::mark_through_ref<T>(p, _queue, _mark_context, false);
}
class ShenandoahSTWMarkTask : public AbstractGangTask {
@ -131,9 +131,10 @@ void ShenandoahSTWMark::finish_mark(uint worker_id) {
ShenandoahPhaseTimings::Phase phase = _full_gc ? ShenandoahPhaseTimings::full_gc_mark : ShenandoahPhaseTimings::degen_gc_stw_mark;
ShenandoahWorkerTimingsTracker timer(phase, ShenandoahPhaseTimings::ParallelMark, worker_id);
ShenandoahReferenceProcessor* rp = ShenandoahHeap::heap()->ref_processor();
StringDedup::Requests requests;
mark_loop(worker_id, &_terminator, rp,
false /* not cancellable */,
ShenandoahStringDedup::is_enabled() ? ALWAYS_DEDUP : NO_DEDUP);
ShenandoahStringDedup::is_enabled() ? ALWAYS_DEDUP : NO_DEDUP, &requests);
}

View File

@ -62,6 +62,19 @@ package gc.stringdedup;
* @run driver gc.stringdedup.TestStringDeduplicationInterned Parallel
*/
/*
* @test TestStringDeduplicationInterned
* @summary Test string deduplication of interned strings
* @bug 8029075
* @requires vm.gc.Shenandoah
* @library /test/lib
* @library /
* @modules java.base/jdk.internal.misc:open
* @modules java.base/java.lang:open
* java.management
* @run driver gc.stringdedup.TestStringDeduplicationInterned Shenandoah
*/
/*
* @test TestStringDeduplicationInterned
* @summary Test string deduplication of interned strings