RFR: 8242082: Shenandoah: Purge Traversal mode

Aleksey Shipilev shade at redhat.com
Fri Apr 3 10:36:30 UTC 2020


On 4/3/20 12:16 PM, Roman Kennke wrote:
> Traversal mode has become cumbersome to maintain. Every single
> non-trivial feature required extra work to play with Traversal, and
> usually caused a trail of follow-up issues. And all that for little
> gain. We are transplanting the interesting properties to the upcoming
> I-U mode (which is much less trouble-some), and can ditch Traversal mode
> altogether.
> 
> My original plan was to ditch it after I-U is in, but now I want to do a
> couple of cleanups before I-U and I'm not in the mood to special-case
> Traversal there. Let's get rid of it ahead of time.
> 
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8242082
> Webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8242082/webrev.00/

Good riddance.

*) Do you have any hits if your grep for "traversal" / "Traversal" on the new code? When I was
playing with my own removal, that highlighted some easy things to ditch.

*) The removal of ShenandoahConcurrentRoots::can_do_concurrent_roots seems premature. It mirrors
other {can|should}_do_* things. I understand it would return true all the time, that seems fine.

*) The addition of is_concurrent_mark_in_progress is weird here. Should be deferred to IU?

  88 inline void ShenandoahBarrierSet::storeval_barrier(oop obj) {
  89   if (obj != NULL && ShenandoahStoreValEnqueueBarrier &&
  90       _heap->is_concurrent_mark_in_progress()) {
  91     enqueue(obj);
  92   }
  93 }

*) In ShenandoahForwardedIsAliveClosure::do_object_b, can drop parentheses around the last arg and
put it on new line?

45   shenandoah_assert_not_forwarded_if(NULL, obj,
46                                      (ShenandoahHeap::heap()->is_concurrent_mark_in_progress()))

-- 
Thanks,
-Aleksey



More information about the shenandoah-dev mailing list