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
