RFR: 8242054: Shenandoah: New incremental-update mode
Roman Kennke
rkennke at redhat.com
Sat Apr 4 00:26:31 UTC 2020
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8242054
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8242054/webrev.00/
I rebased this patch on top of "JDK-8242130: Shenandoah: Simplify
arraycopy-barrier dispatching"
Now that we purged traversal-mode, I also added tests for IU mode, by
(mostly) mechanically adding back the tests that I removed with
Traversal mode, and change ShenandoahGCMode=traversal ->
ShenandoahGCMode=iu.
> *) I would prefer for flag checks to be first. It makes it clearer to me that path is only taken in
> some configurations. Example, this:
> if (dest_uninitialized && ShenandoahSATBBarrier) ...
> ...becomes this:
> if (ShenandoahSATBBarrier && dest_uninitialized) ...
Right. Fixed the occurances that I found (the mentioned one is gone with
JDK-8242130.
> *) These blocks should not be removed. At very least we want to make the check for SATB/StoreVal
> barrier conditional on the mode. But other barrier checks should definitely stay.
>
> 47 // Final configuration checks
> 48 SHENANDOAH_CHECK_FLAG_SET(ShenandoahLoadRefBarrier);
> 49 SHENANDOAH_CHECK_FLAG_SET(ShenandoahSATBBarrier);
> 50 SHENANDOAH_CHECK_FLAG_SET(ShenandoahCASBarrier);
> 51 SHENANDOAH_CHECK_FLAG_SET(ShenandoahCloneBarrier);
Ok. I turned it into:
SHENANDOAH_CHECK_FLAG_SET(ShenandoahSATBBarrier ||
ShenandoahStoreValEnqueueBarrier)
which required the macro to do !(name) instead of !name.
> *) This is some local madness:
>
> 84 int gc_state = _heap->gc_state();
> 85 if ((gc_state & ShenandoahHeap::MARKING) != 0 &&
> 86 ShenandoahStoreValEnqueueBarrier &&
> 87 !_heap->marking_context()->allocated_after_mark_start(obj)) {
> 88 ShenandoahUpdateRefsForOopClosure<false, false, true> cl;
> 89 obj->oop_iterate(&cl);
> 90 } else if ((gc_state & ShenandoahHeap::HAS_FORWARDED) != 0) {
> 91 if (skip_bulk_update(cast_from_oop<HeapWord *>(obj))) return;
This should be gone with JDK-8242130, plus in this revision I also made
the clone_barrier() to dispatch in a very similar fashion.
> *) I don't think the rewrite in ShFinalMarkTask is equivalent. See, on the path here:
>
> 262 ShenandoahSATBAndRemarkCodeRootsThreadsClosure tc(&cl,
> 263 ShenandoahStoreValEnqueueBarrier ? &mark_cl : NULL,
> 264 &blobsCl);
>
> ...we used to pass NULL as blobsCL when do_nmethods is false.
Right. Good catch!
> *) Also, why ResourceMark here?
>
> 199 if (_cl != NULL) {
> 200 ResourceMark rm;
> 201 thread->oops_do(_cl, _code_cl);
Because Threads::oops_do() allocates some Resource stuff. It's
everywhere where Thread::oops_do() is called. Should probably be
looked-at separately?
http://cr.openjdk.java.net/~rkennke/JDK-8242054/webrev.02/
Better?
Roman
More information about the shenandoah-dev
mailing list