Request for reviews (L): 6893081: method handle & invokedynamic code needs additional cleanup (post 6815692, 6858164)

[John Rose]

On Dec 14, 2009, at 5:08 PM, Tom Rodriguez wrote:

>> - Should method handle creation trigger the initialization of the destination class, or should the trigger be deferred until (if ever) the MH is called?  Basically, an MH is a new kind of early linkage, but linkage in the JVM does not trigger initialization; the first call does.  So maybe the long is wrong here.  (Will file a bug and fix later!)
>> 
>> - Should the call to the compile broker be repeated just after the initialization barrier?  I'm thinking this might be overkill.  If the target method is not compiled, the interpreter entry will cause it to be compiled.  I'm not understanding the reason the compile broker is called from LinkResolver; it seems like a mere -Xcomp hack.
> 
> It's the heart of -Xcomp which is essentially a hack.  Part of what confused me in looking at the code is  that is_not_initialized() != !is_initialized() which wasn't obvious.
> 
>  bool is_initialized() const              { return _init_state == fully_initialized; }
>  bool is_not_initialized() const          { return _init_state <  being_initialized; }
> 
> The difference is in how they treat the execution of code during initialization and it's at the heart of my original question.  I was concerned that it would rule out compiling some things that we currently compile in Xcomp mode.  I agree that your original change would have exactly the effect you wanted and no more so I'm ok with it, though a comment on it would be nice.  is_not_initialized could probably be renamed too if you could come up with a better name.  I note that many of the existing uses are !is_not_initialized() suggesting that the sense is wrong.  Having not built into a name is usually a bad idea.

Agree.  Here's a minimum commenting:

diff --git a/src/share/vm/interpreter/linkResolver.cpp b/src/share/vm/interpreter/linkResolver.cpp
--- a/src/share/vm/interpreter/linkResolver.cpp
+++ b/src/share/vm/interpreter/linkResolver.cpp
@@ -75,6 +75,8 @@
   _selected_method = selected_method;
   _vtable_index    = vtable_index;
   if (CompilationPolicy::mustBeCompiled(selected_method)) {
+    // This path is unusual, mostly used by the '-Xcomp' stress test mode.
+
     // Note: with several active threads, the mustBeCompiled may be true
     //       while canBeCompiled is false; remove assert
     // assert(CompilationPolicy::canBeCompiled(selected_method), "cannot compile");
@@ -83,6 +85,8 @@
       return;
     }
     if (instanceKlass::cast(selected_method->method_holder())->is_not_initialized()) {
+      // 'is_not_initialized' means not only '!is_initialized', but also that
+      // initialization has not been started yet ('!being_initialized')
       // Do not force compilation of methods in uninitialized classes.
       // Note that doing this would throw an assert later,
       // in CompileBroker::compile_method.


-- John