RFR: 8031639: make dependency management (mostly) ci independent

[Doug Simon]

Hi Roland,

Thanks for the review. Responses inline below:

On Jan 20, 2014, at 5:13 PM, Roland Westrelin <roland.westrelin at oracle.com> wrote:

> Hi Doug,
> 
>> webrev: http://cr.openjdk.java.net/~dnsimon/JDK-8031639/
> 
> More informative assert messages than “oops” would be appreciated.

Sorry, I’ll change the message to something more descriptive.
> 
> dependencies.hpp:
> 
> comment needs to be fixed:
> 249   GrowableArray<int>*       _dep_seen;  // (seen[h->ident] & (1<<dept))

How would you like it changed (I don’t think I modified this from its original). Are you suggesting the comment needs to reflect its usage in both the original and the #ifdef GRAAL world?

> dependencies.cpp:
> 
> 75     if (is_java_primitive(elemt))  return;   // Ex:  int[][]
> align comment with // Ex:  String[][]

Will do.

> 121 void Dependencies::assert_call_site_target_value(jobject call_site, jobject method_handle) {
> 122   Klass* ctxk = JNIHandles::resolve(call_site)->klass();
> 123   check_ctxk(ctxk);
> 124   assert_common_2(call_site_target_value, DepValue(_oop_recorder, call_site), DepValue(_oop_recorder, method_handle));
> 125 }
> 
> Don’t you need a VM_ENTRY?

Actually, the signature and implementation in this patch is a little of our date with respect to the current Graal code base. It’s now:

  void Dependencies::assert_call_site_target_value(oop call_site, oop method_handle) {
    assert_common_2(call_site_target_value, DepValue(_oop_recorder, JNIHandles::make_local(call_site)), DepValue(_oop_recorder, JNIHandles::make_local(method_handle)));
  }

This is only called from a point within a VM_ENTRY so I don’t think it needs another VM_ENTRY marker, right? Is there someway to assert whether code is within a VM_ENTRY?

> Have you verified that compilation time is not affected?

No. Can you please suggest a reliable way to do this? I’m assuming something like CompileTheWorld?

> I don’t really feel comfortable with this change. I wonder if it wouldn’t be simpler to have a base class for Dependencies, then one subclass for Dependencies that work on ci objects and one subclass that work directly on objects and Metadata. Then either accept code duplication or maybe use a parallel hierarchy for DepValue.

If the compilation time is not impacted, wouldn’t it be better to avoid code duplication altogether?

I’ll post another webrev after waiting a bit for further feedback.

-Doug