Hi Serguei, On 6/12/2016 3:57 PM, [email protected] 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; }
This saves _exception_state into *state.
+ inline void restore_exception_state(ExceptionState state) {
_exception_state = state; }
This restores _exception_state from state.
The functions above do not really encapsulate and save/restore the _exception_state value so that these
I think they do. Cheers, David -----
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 agentWhat the 'ks' stands for? I have some guess but still would like to know this abbreviation. Thanks, Serguei4. RBT hotspot_all -Dmitry
