[wasm] Fix mprotect calls, lock contention of write protection

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}

src/wasm/wasm-code-manager.cc

@@ -788,12 +788,6 @@ void WasmCodeAllocator::FreeCode(Vector<WasmCode* const> codes) {
   size_t code_size = 0;
   CODE_SPACE_WRITE_SCOPE
   for (WasmCode* code : codes) {
-    // TODO(dlehmann): Pull the {NativeModuleModificationScope} out of the loop.
-    // However, its constructor requires a {NativeModule}.
-    // Can be fixed if {NativeModuleModificationScope()} is changed to take
-    // only a {WasmCodeAllocator} in its constructor.
-    NativeModuleModificationScope native_module_modification_scope(
-        code->native_module());
     ZapCode(code->instruction_start(), code->instructions().size());
     FlushInstructionCache(code->instruction_start(),
                           code->instructions().size());
@@ -1033,11 +1027,11 @@ void NativeModule::UseLazyStub(uint32_t func_index) {
             module_->num_imported_functions + module_->num_declared_functions);
 
   base::RecursiveMutexGuard guard(&allocation_mutex_);
+  NativeModuleModificationScope native_module_modification_scope(this);
   if (!lazy_compile_table_) {
     uint32_t num_slots = module_->num_declared_functions;
     WasmCodeRefScope code_ref_scope;
     CODE_SPACE_WRITE_SCOPE
-    NativeModuleModificationScope native_module_modification_scope(this);
     DCHECK_EQ(1, code_space_data_.size());
     base::AddressRegion single_code_space_region = code_space_data_[0].region;
     lazy_compile_table_ = CreateEmptyJumpTableInRegionLocked(
@@ -1074,6 +1068,7 @@ std::unique_ptr<WasmCode> NativeModule::AddCode(
     jump_table_ref =
         FindJumpTablesForRegionLocked(base::AddressRegionOf(code_space));
   }
+  NativeModuleModificationScope native_module_modification_scope(this);
   return AddCodeWithCodeSpace(index, desc, stack_slots, tagged_parameter_slots,
                               protected_instructions_data,
                               source_position_table, kind, tier, for_debugging,
@@ -1101,7 +1096,6 @@ std::unique_ptr<WasmCode> NativeModule::AddCodeWithCodeSpace(
   const int instr_size = desc.instr_size;
 
   CODE_SPACE_WRITE_SCOPE
-  NativeModuleModificationScope native_module_modification_scope(this);
   base::Memcpy(dst_code_bytes.begin(), desc.buffer,
                static_cast<size_t>(desc.instr_size));
 
@@ -1153,6 +1147,7 @@ WasmCode* NativeModule::PublishCode(std::unique_ptr<WasmCode> code) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
                "wasm.PublishCode");
   base::RecursiveMutexGuard lock(&allocation_mutex_);
+  NativeModuleModificationScope native_module_modification_scope(this);
   return PublishCodeLocked(std::move(code));
 }
 
@@ -1163,6 +1158,9 @@ std::vector<WasmCode*> NativeModule::PublishCode(
   std::vector<WasmCode*> published_code;
   published_code.reserve(codes.size());
   base::RecursiveMutexGuard lock(&allocation_mutex_);
+  // Get writable permission already here (and not inside the loop in
+  // {PatchJumpTablesLocked}), to avoid switching for each {code} individually.
+  NativeModuleModificationScope native_module_modification_scope(this);
   // The published code is put into the top-most surrounding {WasmCodeRefScope}.
   for (auto& code : codes) {
     published_code.push_back(PublishCodeLocked(std::move(code)));
@@ -1271,6 +1269,7 @@ void NativeModule::ReinstallDebugCode(WasmCode* code) {
   code_table_[slot_idx] = code;
   code->IncRef();
 
+  NativeModuleModificationScope native_module_modification_scope(this);
   PatchJumpTablesLocked(slot_idx, code->instruction_start());
 }
 
@@ -1383,7 +1382,6 @@ void NativeModule::PatchJumpTablesLocked(uint32_t slot_index, Address target) {
   allocation_mutex_.AssertHeld();
 
   CODE_SPACE_WRITE_SCOPE
-  NativeModuleModificationScope native_module_modification_scope(this);
   for (auto& code_space_data : code_space_data_) {
     DCHECK_IMPLIES(code_space_data.jump_table, code_space_data.far_jump_table);
     if (!code_space_data.jump_table) continue;
@@ -2104,6 +2102,9 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
 
   // Now copy the generated code into the code space and relocate it.
   CODE_SPACE_WRITE_SCOPE
+  // Get writable permission already here (and not inside the loop in
+  // {AddCodeWithCodeSpace}), to avoid lock contention on the
+  // {allocator_mutex_} if we try to switch for each code individually.
   NativeModuleModificationScope native_module_modification_scope(this);
   for (auto& result : results) {
     DCHECK_EQ(result.code_desc.buffer, result.instr_buffer.get());
@@ -2163,6 +2164,9 @@ std::vector<int> NativeModule::FindFunctionsToRecompile(
     TieringState new_tiering_state) {
   WasmCodeRefScope code_ref_scope;
   base::RecursiveMutexGuard guard(&allocation_mutex_);
+  // Get writable permission already here (and not inside the loop in
+  // {PatchJumpTablesLocked}), to avoid switching for each slot individually.
+  NativeModuleModificationScope native_module_modification_scope(this);
   std::vector<int> function_indexes;
   int imported = module()->num_imported_functions;
   int declared = module()->num_declared_functions;
@@ -2199,6 +2203,10 @@ std::vector<int> NativeModule::FindFunctionsToRecompile(
 
 void NativeModule::FreeCode(Vector<WasmCode* const> codes) {
   base::RecursiveMutexGuard guard(&allocation_mutex_);
+  // Get writable permission already here (and not inside the loop in
+  // {WasmCodeAllocator::FreeCode}), to avoid switching for each {code}
+  // individually.
+  NativeModuleModificationScope native_module_modification_scope(this);
   // Free the code space.
   code_allocator_.FreeCode(codes);