On Tue, 17 May 2022 14:54:23 GMT, Johannes Bechberger <d...@openjdk.java.net> wrote:
>> But you aren't keeping it on the stack versus TLS you are using a stack >> object to set the TLS. All we have done in this final form is replace: >> >> >> thread->set_in_asgct(true); >> <stuff> >> thread->set_in_asgct(false); >> >> with >> >> >> ASGCTMark mark(thread); >> <stuff> >> >> but now ASGCTMark has to worry about whether `thread` is NULL, whether it is >> the current javaThread, when we've already done all that for `thread`. >> >> And this is the only place we will ever use ASGCTMark. Now if we had lots of >> return points in `<stuff>` then RAII would definitely be better to ensure we >> can't forget to reset the state. But we don't. So use of RAII adds >> complexity cost now just in case in the future RAII might be useful. > > But the crash-handler does not prevent that `guarantee` aborts the VM, as we > discussed already in my related PR. My comment was more a suggestion and > related to "And this is the only place we will ever use ASGCTMark" by > @dholmes-ora . I agree with you that changing JFR should be done in another > PR. > AFAIK, currently JFR is 'wrapping' the code in a crash-handler. We can take a > look at reusing this approach there in some follow-up PR but for now I would > really prefer getting this one merged without attaching any more > bells&whistles. @jbachorik No, please don't do that. That approach is very unsafe and should be used with extreme care. I would actually prefer for it to get removed completely, not to be reused. ------------- PR: https://git.openjdk.java.net/jdk/pull/8549
