On Mon, 7 Mar 2022 11:29:08 GMT, Johannes Bechberger <d...@openjdk.java.net> 
wrote:

> The WXMode for the current thread (on MacOS aarch64) is currently stored in 
> the thread class which is unnecessary as the WXMode is bound to the current 
> OS thread, not the current instance of the thread class.
> This pull request moves the storage of the current WXMode into a thread local 
> global variable in `os` and changes all related code. SafeFetch depended on 
> the existence of a thread object only because of the WXMode. This pull 
> request therefore removes the dependency, making SafeFetch usable in more 
> contexts.

Hi Johannes,

The general idea seems good (pity it touches so many files, but then I've never 
liked any of this WX support precisely because it is so invasive of shared 
code). I agree that safeFetch should not have become dependent on 
Thread::current existing, but I have to wonder whether we can just skip the WX 
code if there is no current thread? If the thread is not attached to the VM 
then what does it even mean to manipulate the WX state of an unknown thread?

That aside, with this change I think we can move the conditional WX code out of 
the shared os.hpp and bury it down in os_bsd_aarch64.hpp where it actually 
belongs. 

I'd even like to see threadWXSetters.inline.hpp moved to being in 
src/os_cpu/bsd_aarch64/ if feasible - I'm not sure what include would be needed 
for the callsites to function - os.hpp I presume?

Thanks,
David

src/hotspot/share/runtime/threadWXSetters.inline.hpp line 33:

> 31: #if defined(__APPLE__) && defined(AARCH64)
> 32: 
> 33: #include "runtime/thread.inline.hpp" // dependencies require this include

I can't see how this include is needed now.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7727

Reply via email to