RFR: 8069016: Add BarrierSet downcast support

[Kim Barrett]

Jon, thanks for looking at this.

On Feb 20, 2015, at 3:04 AM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
> 
> 
> http://cr.openjdk.java.net/~kbarrett/8069016/webrev.00/src/share/vm/utilities/fakeRttiSupport.hpp.html
> 
>  86   TagType _ctag;
> 
> I'd prefer that the variable be named _concrete_tag

Sure, I'll change to _concrete_tag.

> http://cr.openjdk.java.net/~kbarrett/8069016/webrev.00/src/share/vm/gc_implementation/parallelScavenge/cardTableExtension.hpp.udiff.html
> 
> +      // Concrete tag should be BarrierSet::CardTableExtension.
> +      // That will presently break things in a bunch of places though.
> 
> Can you expand on these comments.  Where would the breakage occur?
> 
> 
> http://cr.openjdk.java.net/~kbarrett/8069016/webrev.00/src/share/vm/gc_implementation/parallelScavenge/psMarkSweep.cpp.udiff.html
> 
> -    BarrierSet* bs = heap->barrier_set();
> -    if (bs->is_a(BarrierSet::ModRef)) {
> -      ModRefBarrierSet* modBS = (ModRefBarrierSet*)bs;
> +    ModRefBarrierSet* modBS = barrier_set_cast<ModRefBarrierSet>(heap->barrier_set());
> 
> 
> Is this one of the place that breaks because the barrier set is really a CardTableExtension?
> 
> http://cr.openjdk.java.net/~kbarrett/8069016/webrev.00/src/share/vm/memory/cardTableModRefBS.hpp.udiff.html
> 
> +      // Concrete tag should be BarrierSet::CardTableForRS.
> +      // That will presently break things in a bunch of places though.
> 
> 
> Expand on this comment also about what will break.

The concrete tag is used as a dispatch key in many places.

I'm not sure all of those places handle BarrierSet::CardTableExtension
properly.  One place that I think would probably "only" be a
performance regression is barrierSet.inline.hpp, where the concrete
tag is used to decide whether to devirtualize the call.

Since BarrierSet::CardTableForRS is new and not propogated everywhere,
using it as the concrete tag would certainly break things badly for
relevant collectors.

There is a CR for the CardTableExtension case:
https://bugs.openjdk.java.net/browse/JDK-8072817

My current thinking is to address some of this by modifications to the
barrier set class hierarchy and the elimination of the use of the
concrete tag in favor of (now fast) is_a() tests.  For example, change
the devirtualization in barrierSet.inline.hpp to use
is_a(BarrierSet::CardTableModRef).  That doesn't presently work,
because the G1 barrier sets are derived from there; changing that
would be one of the class hierarchy changes I'm thinking about.

A goal of introducing barrier_set_cast is to make such refactorings
easier.