RFR (L): 8134994: use separate VMStructs databases for SA and JVMCI

[Christian Thalinger]

> On Dec 10, 2015, at 1:13 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
> On Dec 9, 2015, at 5:16 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8134994 <https://bugs.openjdk.java.net/browse/JDK-8134994>
>> http://cr.openjdk.java.net/~twisti/8134994/webrev/ <http://cr.openjdk.java.net/~twisti/8134994/webrev/>
>> 
>> Currently JVMCI uses the existing VMStructs database which the SA also uses.  In order to be more flexible with the future of the SA the JVMCI should use it's own database.
> 
> Thanks for doing this.
> 
> My hope is that, eventually, all the friendship with JVMCIVMStructs will
> be eliminated, and JVMCI will only be relying on public APIs that the
> various components provide for that purpose.  But this is a good
> start, giving much better visibility into what JVMCI is using.
> 
> ------------------------------------------------------------------------------
> src/share/vm/gc/g1/ptrQueue.hpp 
>  41 class PtrQueue VALUE_OBJ_CLASS_SPEC {
>  42   friend class VMStructs;
>  43   friend class JVMCIVMStructs;
> 
> I thought I'd fixed things so JVMCI shouldn't need PtrQueue
> friendship.  I think the APIs it needs and uses are public in
> SATBMarkQueue and DirtyCardQueue.
> 
> Oh, but some of the relevant code has been removed from
> src/share/vm/gc/g1/vmStructs_g1.hpp, and I don't (yet) see it having
> been relocated.  Indeed, it looks like at least part of my change has
> been lost, since we are back to
> 
> src/share/vm/jvmci/vmStructs_jvmci.cpp
> 207   nonstatic_field(PtrQueue,                    _active,                                       bool)                                  \
> 208   nonstatic_field(PtrQueue,                    _buf,                                          void**)                                \
> 209   nonstatic_field(PtrQueue,                    _index,                                        size_t)                                \
> 
> Here's the change that seems to have been (partially?) undone:
> https://bugs.openjdk.java.net/browse/JDK-8143014
> Access PtrQueue member offsets through derived classes
> 
> But searching through vmStructs_jvmci.cpp, it looks like the only
> problem might be that the PtrQueue friendship and the PtrQueue field
> access in vmStructs_jvmci.cpp are not needed and should be removed.

Correct.  I’ve had this working copy with the new file for a while and so it was still around.  Thanks for catching that.

> 
> ------------------------------------------------------------------------------
> src/share/vm/gc/shared/cardTableModRefBS.hpp
>  43 class CardTableModRefBS: public ModRefBarrierSet {
>  44   // Some classes get to look at some private stuff.
>  45   friend class VMStructs;
>  46   friend class JVMCIVMStructs;
> 
> I thought we'd eliminated the need for this friendship for JVMCI too,
> though I'm less certain in this case.  Maybe its just been discussed
> how it could be done, but hasn't been done yet?

The friendship exists because of dirty_card which is protected but I can fix that with declare_constant_with_value.  I got rid of the friendship in G1SATBCardTableModRefBS also.

> 
> ------------------------------------------------------------------------------ 
> src/share/vm/jvmci/jvmciCompilerToVM.cpp
> 186     jbyte* base = ((CardTableModRefBS*)bs)->byte_map_base;
> 
> Use barrier_set_cast<CardTableModRefBS>(bs), so that when we refactor
> the barrier set hierarchy (which we plan to do), this will break in an
> obvious way.

Done.

> 
> And instead of the switch on bs->kind(), maybe use
> 
>  if (bs->is_a(BarrierSet::CardTableModRefBS)) {
>    ... ;
>  } else {
>    ShouldNotReachHere();
>  }
> 
> All current implementations of BarrierSet are CTMRBS.  If/when that's
> no longer true, this will break in an obvious way.

What about:

  case BarrierSet::ModRef:

?

http://cr.openjdk.java.net/~twisti/8134994/webrev.02/ <http://cr.openjdk.java.net/~twisti/8134994/webrev.02/>

(the previous version is now under: http://cr.openjdk.java.net/~twisti/8134994/webrev.01/ <http://cr.openjdk.java.net/~twisti/8134994/webrev.01/>)

> 
> ------------------------------------------------------------------------------
>