On Sat, 1 Nov 2025 09:01:58 GMT, Andrew Haley <[email protected]> wrote:
>> Re: SafeFetch, it is probably OK to make NativePostCallNop_at slightly >> slower for uses like make_deoptimized(), but the oopmap optimizations like >> CodeCache::find_blob_and_oopmap() were highly optimized to make >> loom/VirtualThread performance reasonable. Adding a SafeFetch here might >> cause a regression. > >> Re: SafeFetch, it is probably OK to make NativePostCallNop_at slightly >> slower for uses like make_deoptimized(), but the oopmap optimizations like >> CodeCache::find_blob_and_oopmap() were highly optimized to make >> loom/VirtualThread performance reasonable. Adding a SafeFetch here might >> cause a regression. > > Sure, but 2 things: > Loom doesn't meed post-call NOPs as much as it used to. > We could fairly easily make SafeFetch much faster than it is, if needs be. > But anyway, I approved this patch. Thank you for the detailed advice, @theRealAph, I now see how `SafeFetch` can be valuable independently of whether false-positive matches with the post-call NOP pattern can happen during normal execution. I hadn't considered the stack corruption use case before. Reviewing the `SafeFetch` implementation, I believe in general case it relies on `sigsetjmp` on POSIX systems and exceptions on Windows. However, for AArch64, the `SafeFetch32` has an optimized implementation - avoiding `setjmp` or exceptions overhead. On the fast path, it performs just one load, so any extra performance cost would be due to that path cannot currently be inlined. There indeed seems to be a way to have it inlined, at least on Linux - via creating an extra ELF section containing addresses of all inlined `SafeFetch` loads and corresponding continuation points, which the signal handler can iterate through. I've not prototyped this, but if feasible, it could make the performance impact of using `SafeFetch` negligible. Since there isn't necessarily a consensus at this stage on whether `SafeFetch` should be added in this PR, I'd propose opening a separate JBS ticket for it to avoid blocking merge of the exception handler stub code cleanup. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26678#issuecomment-3479653271
