-
Daniel Lehmann authored
For mprotect-based write protection of WebAssembly code memory, we open {NativeModuleModificationScope}s each time a thread needs write-access to the code space. While fine-grained switching is good for security (the permission should only be granted for as short as possible, especially since it is process-wide), this can degrade performance considerably for two reasons (we measured up to 10x slower Liftoff compilation time cf. having no write protection): 1. Switching permissions with mprotect() (and likely with similar functions on non-POSIX platforms) is just inherently expensive due to the syscall, modifying page tables, and potentially subsequent TLB flushes. For a simple benchmark (compiling Unity with --liftoff-only) --wasm-write-protect-code-memory increases the number of mprotect syscalls from ~2.6-2.8k to 6-8k (!). 2. Modifying the permissions in {SetWritable()} is synchronized across threads via the {NativeModule::allocator_mutex_}. With many fine- grained permission switching requests, lock contention on this mutex incurs a very high number of futex syscalls (measured on Linux only, but the problem is likely a general one). For the same simple benchmark as above (compiling Unity), --wasm-write-protect-code-memory increases the number of futex syscalls from ~1k to 20-40k (!). Both problems are fixed in the CL here, following this simple recipe (in case we get more of these issues in the future): 1. Identify the hot syscall either via sampling-based profiling with `sudo perf record -g -F10000 d8 ...` (needs sudo for kernel stacks) and then looking into the record or a flamegraph, or with event-based profiling with `sudo perf stat -g -e 'syscalls:sys_enter*' d8 ...`. In particular, if {NativeModuleModificationScope}s are repeatedly opened (behind a function) in a loop, this can be a problem. 2. Add a scope object outside of the loop, potentially to a function upwards in the call hierarchy of the hot loop/function. 3. Remove the scope object in the innermost function/hot loop. 4. Check all callers of the hot function (which now no longer has a scope object), whether additional scopes need to be added there for correctness. The following two offenders were especially visible in the profile: - Most of the mprotect calls were coming from {PatchJumpTablesLocked}. Pulled the scope object up into {PublishCode}. - Most of the lock contention was caused by {AddCodeWithCodeSpace}. There already was a scope object up the call chain in {AddCompiledCode}. - Fixed scope inside the loop in {FreeCode} for good measure as well. R=clemensb@chromium.org CC=jkummerow@chromium.org Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel_ng Bug: v8:11663, chromium:932033 Change-Id: I89e4a1f0998f06e4d4b5e360e0bf81836d4240f7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2912786 Commit-Queue: Daniel Lehmann <dlehmann@google.com> Reviewed-by: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#74763}
bdd4a6b7