• Daniel Lehmann's avatar
    [wasm] Fix mprotect calls, lock contention of write protection · bdd4a6b7
    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: 's avatarClemens Backes <clemensb@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#74763}
    bdd4a6b7
wasm-code-manager.cc 95.6 KB