On Thu, 10 Mar 2022 12:41:11 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
> You are saying that Java Threads may write too. And CompilerThreads, in the > case of SafeFetch at least, may run generated code. So switching has to be > done as a stack mark. Have I gotten this right? Right. > Depending on what the pthread library call does, and if it's a real function > call into a library, it would be more expensive than that. Yes, unfortunately we need something like this. > > If we compare approaches in general (not only SafeFetch), the Thread is > > already in hands, so we should to compare a read of the field from a C++ > > object, and the read of a TLS variable. The former could not be slower than > > the latter. > > To me most of the invocations of `ThreadWXEnable` seem to use > `Thread::current()`. Only those who retrieve the thread from the JNI > environment don't. Right, JNI env is used e.g. in interfaceSupport.hpp where the most VM entries are defined. I found only few instances of ThreadWXEnable to receive Thread::current() as the argument immediately. In the rest, the Thread is there somewhere in the context. > > IIRC, TLS, at least on Linux, lives at the front of the thread stack, so > accessing it should be quite cheap. > > I see the performance point of an option to pass in Thread* in case one > already has it. I dislike it a bit because it gives the illusion that you > could pass in arbitrary threads when in fact you must pass in > Thread::current. But an assertion could help clarifying here. There is the assert in Thread::enable_wx, where the implementation actually is unable to handle anything except the current threads. > > Is it possible to change SafeFetch only? Switch to WXExec before calling > > the stub and switch WXWrite back unconditionally? We won't need to provide > > assert in ThreadWXEnable. But SafeFetch can check the assumption with > > assert via Thread, if it exists. > > But SafeFetch could be used from outside code as well as VM code. In case of > the latter, prior state can either be WXWrite or WXExec. It needs to restore > the prior state after the call. I'm not sure I understand what is the "outside code". The SafeFetch is the private hotspot function, it cannot be linked with non-JVM code, isn't it? > To summarize the different proposals: > > * you propose to use Thread* when available and assume WXWrite as prior state > when not. You argue that if there is no Thread::current, we are not a VM > thread and we should need no nesting, so a simple switchback to wxwrite > should suffice after leaving SafeFetch, right? So far I like the another approach more, that is, always assume WXWrite and use the Thread only to assert the state. But I did not understand your concern above. > * Johannes proposes to use TLS, and just always support nesting, regardless > of who calls. > > What I like about Johannes proposal is that its simple. It has fewer > dependencies on VM infrastructure and we can mostly just hide it in the > platform tree. I'm not opposing the refactoring, it has some advantages, but I'd want to separate functional change from the refactoring. This would aid backporting, at least. ------------- PR: https://git.openjdk.java.net/jdk/pull/7727
