[wasm] Only make needed regions writable

On Windows, the overhead of {SetPermissions} (which maps to a
{VirtualAlloc} call) heavily depends on the amount of memory on which
permissions are switched. Hence this CL changes permission switching
to only switch the code regions that are actually needed. This will
increase the number of system calls, but reduce the total size of
switched memory.

On a Unity benchmark, this reduced the lazy compilation time on Windows
from 13.7 seconds to 3.6 seconds (3.0 seconds without write protection).
On Linux, there is no measurable effect, but permission switching
generally seems to have way less overhead on Linux.

R=jkummerow@chromium.org

Bug: v8:11974
Change-Id: I46dd4ae9997587226b3d81166cf2e1128383ab34
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3077144
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76163}

src/wasm/code-space-access.cc

@@ -54,7 +54,7 @@ void CodeSpaceWriteScope::SetWritable() const {
     DCHECK(FLAG_wasm_memory_protection_keys);
     code_manager->SetThreadWritable(true);
   } else if (FLAG_wasm_write_protect_code_memory) {
-    CHECK(native_module_->SetWritable(true));
+    native_module_->AddWriter();
   }
 }
 
@@ -64,7 +64,7 @@ void CodeSpaceWriteScope::SetExecutable() const {
     DCHECK(FLAG_wasm_memory_protection_keys);
     code_manager->SetThreadWritable(false);
   } else if (FLAG_wasm_write_protect_code_memory) {
-    CHECK(native_module_->SetWritable(false));
+    native_module_->RemoveWriter();
   }
 }
 

src/wasm/wasm-code-manager.h

@@ -508,10 +508,19 @@ class WasmCodeAllocator {
   base::Vector<byte> AllocateForCodeInRegion(NativeModule*, size_t size,
                                              base::AddressRegion);
 
-  // Sets permissions of all owned code space to read-write or read-only (if
-  // {writable} is false). Returns true on success.
+  // Increases or decreases the {writers_count_} field. While there is at least
+  // one writer, it is allowed to call {MakeWritable} to make regions writable.
+  // When the last writer is removed, all code is switched back to
+  // write-protected.
+  // Hold the {NativeModule}'s {allocation_mutex_} when calling one of these
+  // methods. The methods should only be called via {CodeSpaceWriteScope}.
+  V8_EXPORT_PRIVATE void AddWriter();
+  V8_EXPORT_PRIVATE void RemoveWriter();
+
+  // Make a code region writable. Only allowed if there is at lease one writer
+  // (see above).
   // Hold the {NativeModule}'s {allocation_mutex_} when calling this method.
-  V8_EXPORT_PRIVATE bool SetWritable(bool writable);
+  void MakeWritable(base::AddressRegion);
 
   // Free memory pages of all given code objects. Used for wasm code GC.
   // Hold the {NativeModule}'s {allocation_mutex_} when calling this method.
@@ -527,6 +536,9 @@ class WasmCodeAllocator {
   static constexpr base::AddressRegion kUnrestrictedRegion{
       kNullAddress, std::numeric_limits<size_t>::max()};
 
+  void InsertIntoWritableRegions(base::AddressRegion region,
+                                 bool switch_to_writable);
+
   //////////////////////////////////////////////////////////////////////////////
   // These fields are protected by the mutex in {NativeModule}.
 
@@ -540,11 +552,15 @@ class WasmCodeAllocator {
   DisjointAllocationPool freed_code_space_;
   std::vector<VirtualMemory> owned_code_space_;
 
+  // The following two fields are only used if {protect_code_memory_} is true.
   int writers_count_{0};
+  std::set<base::AddressRegion, base::AddressRegion::StartAddressLess>
+      writable_memory_;
 
   // End of fields protected by {mutex_}.
   //////////////////////////////////////////////////////////////////////////////
 
+  const bool protect_code_memory_;
   std::atomic<size_t> committed_code_space_{0};
   std::atomic<size_t> generated_code_size_{0};
   std::atomic<size_t> freed_code_size_{0};
@@ -657,9 +673,19 @@ class V8_EXPORT_PRIVATE NativeModule final {
   // to a function index.
   uint32_t GetFunctionIndexFromJumpTableSlot(Address slot_address) const;
 
-  bool SetWritable(bool writable) {
+  void AddWriter() {
+    base::RecursiveMutexGuard guard{&allocation_mutex_};
+    code_allocator_.AddWriter();
+  }
+
+  void RemoveWriter() {
+    base::RecursiveMutexGuard guard{&allocation_mutex_};
+    code_allocator_.RemoveWriter();
+  }
+
+  void MakeWritable(base::AddressRegion region) {
     base::RecursiveMutexGuard guard{&allocation_mutex_};
-    return code_allocator_.SetWritable(writable);
+    code_allocator_.MakeWritable(region);
   }
 
   // For cctests, where we build both WasmModule and the runtime objects

src/wasm/wasm-serialization.cc

@@ -650,6 +650,7 @@ bool NativeModuleDeserializer::Read(Reader* reader) {
 
   std::vector<DeserializationUnit> batch;
   const byte* batch_start = reader->current_location();
+  CodeSpaceWriteScope code_space_write_scope(native_module_);
   for (uint32_t i = first_wasm_fn; i < total_fns; ++i) {
     DeserializationUnit unit = ReadCode(i, reader);
     if (!unit.code) continue;