Deneau, Tom wrote:
Cross posting to hotspot-compiler-...@openjdk.java.net and
serviceability-dev@openjdk.java.net since this review request touches
both compiler/runtime code and JVMTI code...

New webrev is at http://cr.openjdk.java.net/~tdeneau/6902182/webrev.04

src/share/vm/c1/c1_Runtime1.cpp
   No comments.
src/share/vm/opto/graphKit.cpp
   No comments.

src/share/vm/opto/graphKit.hpp
   No comments.

src/share/vm/opto/parse2.cpp
   Comment on line 2069 seems redundant.

src/share/vm/opto/runtime.cpp
   Typo: 'if we exceptions' -> 'if exceptions'

src/share/vm/prims/jvmtiEventController.cpp
   The JvmtiEventControllerPrivate::recompute_thread_enabled()
   change breaks the JavaThread versus JvmtiThreadState
   abstraction a bit.

   I would have done lines 516-517 like:

       bool should_post_on_exceptions =
           (any_env_enabled & SHOULD_POST_ON_EXCEPTIONS_BITS) != 0;
       state->set_should_post_on_exceptions(should_post_on_exceptions);

   This would require a new function in JvmtiThreadState named
   set_should_post_on_exceptions() that makes the actual call
   to _thread->set_should_post_on_exceptions_flag(). The fact
   that the new flag lives in the JavaThread is an implementation
   detail that the JvmtiEvent stuff shouldn't care about.

src/share/vm/prims/jvmtiExport.cpp
   No comments.

src/share/vm/prims/jvmtiExport.hpp
   No comments.

src/share/vm/runtime/thread.cpp
   No comments.

src/share/vm/runtime/thread.hpp
   No comments.



Note to Dan:

   * We had talked about changing the name of the existing
     jvmti_can_post_exceptions() to jvmti_can_post_on_exceptions() to
     make it match the new should_post_on_exceptions name used above.
     Since this affected some other files, to keep this webrev
     simpler, I decided not to do that as part of this webrev.  If we
     still want to do this, I can post this additional change as the
     final webrev.


Following the existing style would be clearer (IMHO):

   can_post_foo <=> should_post_foo

so it should be:

   can_post_on_exceptions <=> should_post_on_exceptions

in the JvmtiExport stuff. Looks like the compiler's
jvmti_can_post_exceptions() should also change to jvmti_can_post_on_exceptions() to match.

Since you correctly pointed out that it really should be
"post *on* exceptions" instead of "post exceptions", I would
prefer if the change were made as part of this work. However,
I can make the change later if you wish.

Dan

Reply via email to