Dmitry,

On 12/6/16 01:54, Dmitry Samersoff wrote:
Serguei.

I intentionally decide not to provide generic accessor functions
for _exception_state.

Unfortunately, provided functions are the same functionally.



Developers should use set_exception_detected(),  set_exception_caught(),
  clear_exception_state() but don't change _exception_state directly.

And save_exception_state()/restore_exception_state() name and signature
clear indicates that the only valid usage of these functions is backup
of exception_state.

So I would prefer to leave it as is.

No pressure.


Thanks,
Serguei


-Dmitry


On 2016-12-06 08:57, serguei.spit...@oracle.com wrote:
Hi Dmitry,


I'm repeating my comments from the internal review.

It looks pretty good to me.
Thank you for the work on it!

I'm not a big fun of new jvmtiThreadState functions though:

+ inline void save_exception_state(ExceptionState *state) { *state =
_exception_state; }
+ inline void restore_exception_state(ExceptionState state) {
_exception_state = state; } The functions above do not really
encapsulate and save/restore the _exception_state value so that these
names are kind of misleading. The same functionality can be provided
with a simplified approach:     inline JvmtiThreadState
exception_state() { return_exception_state;  }
    inline void set_exception_state(ExceptionState state){
_exception_state = state; }

I do not want to insist on my approach and will wait for other people
comments on this.
I hope, some compromise can be found here.


On 12/5/16 01:30, Dmitry Samersoff wrote:
Everybody,

Please review.

http://cr.openjdk.java.net/~dsamersoff/JDK-8165496/webrev.07/

Bug link:

https://bugs.openjdk.java.net/browse/JDK-8165496

Two separate flags, exception_detected and exception_caught, replaced
with one that changes it's state.

Assert assert(_exception_caught == false) and fix for JDK-8134434 are
removed.

Testing:
    1. Manual testing of instrumented hotspot
    2. Local ks with stress agent
    3. RBT ks with stress agent
What the 'ks' stands for?
I have some guess but still would like to know this abbreviation.

Thanks,
Serguei

    4. RBT hotspot_all

-Dmitry



Reply via email to