[wasm] Remove fine-grained CodeSpaceMemoryModificationScopes

In a recent CL I introduced module-level CodeSpaceMemoryModificationScopes,
which means that the number of CodeSpaceMemoryModificationScopes which
are opened aside from TurboFan compilation is not linear in the number
of functions anymore. In that CL, however, I did not remove scopes which
became obsolete. This CL removes now these obsolete scopes, and
introduces some scopes where shared code was used from outside the
compilation logic.

R=mstarzinger@chromium.org, clemensh@chromium.org

Bug: chromium:787731
Change-Id: I37d514efa3a4b10adb7008986a9c91e4557ce618
Reviewed-on: https://chromium-review.googlesource.com/790490
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49662}

src/compiler/wasm-compiler.cc

@@ -4332,9 +4332,6 @@ void AttachWasmFunctionInfo(Isolate* isolate, Handle<Code> code,
   }
   deopt_data->set(1, Smi::FromInt(func_index));
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-
   code->set_deoptimization_data(*deopt_data);
 }
 
@@ -4423,8 +4420,6 @@ Handle<Code> CompileWasmToJSWrapper(
     Handle<Object> index_handle = isolate->factory()->NewNumberFromInt(
         OffsetForImportData(index, WasmGraphBuilder::kFunction));
     deopt_data->set(1, *index_handle);
-    // TODO(6792): No longer needed once WebAssembly code is off heap.
-    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
     code->set_deoptimization_data(*deopt_data);
 #ifdef ENABLE_DISASSEMBLER
     if (FLAG_print_opt_code && !code.is_null()) {
@@ -4573,9 +4568,6 @@ Handle<Code> CompileWasmInterpreterEntry(Isolate* isolate, uint32_t func_index,
     }
   }
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-
   Handle<FixedArray> deopt_data = isolate->factory()->NewFixedArray(1, TENURED);
   Handle<WeakCell> weak_instance = isolate->factory()->NewWeakCell(instance);
   deopt_data->set(0, *weak_instance);
@@ -4933,8 +4925,6 @@ void WasmCompilationUnit::PackProtectedInstructions(Handle<Code> code) const {
     fn_protected->set(Code::kTrapDataSize * i + Code::kTrapLandingOffset,
                       Smi::FromInt(instruction.landing_offset));
   }
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate_->heap());
   code->set_protected_instructions(*fn_protected);
 }
 

src/heap/heap.cc

@@ -3211,7 +3211,6 @@ AllocationResult Heap::AllocateCode(
 }
 
 AllocationResult Heap::CopyCode(Code* code, CodeDataContainer* data_container) {
-  CodeSpaceMemoryModificationScope code_modification(this);
   AllocationResult allocation;
 
   HeapObject* result = nullptr;

src/wasm/module-compiler.cc

@@ -238,8 +238,6 @@ class JSToWasmWrapperCache {
     int cached_idx = sig_map_.Find(func->sig);
     if (cached_idx >= 0) {
       Handle<Code> code = isolate->factory()->CopyCode(code_cache_[cached_idx]);
-      // TODO(6792): No longer needed once WebAssembly code is off heap.
-      CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
       // Now patch the call to wasm code.
       for (RelocIterator it(*code, RelocInfo::kCodeTargetMask);; it.next()) {
         DCHECK(!it.done());
@@ -746,6 +744,8 @@ void LazyCompilationOrchestrator::CompileFunction(
                                      CStrVector(func_name.c_str()), func_index,
                                      CEntryStub(isolate, 1).GetCode());
   unit.ExecuteCompilation();
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   MaybeHandle<Code> maybe_code = unit.FinishCompilation(&thrower);
 
   // If there is a pending error, something really went wrong. The module was
@@ -1449,7 +1449,7 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
   // base::Optional to be able to close the scope before notifying the debugger.
   base::Optional<CodeSpaceMemoryModificationScope> modification_scope(
       base::in_place_t(), isolate_->heap());
-  // The {module> parameter is passed in to transfer ownership of the WasmModule
+  // The {module} parameter is passed in to transfer ownership of the WasmModule
   // to this function. The WasmModule itself existed already as an instance
   // variable of the ModuleCompiler. We check here that the parameter and the
   // instance variable actually point to the same object.
@@ -1565,6 +1565,12 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
   Handle<WasmCompiledModule> compiled_module = NewCompiledModule(
       isolate_, shared, code_table, export_wrappers, env.get());
 
+  // Compile JS->wasm wrappers for exported functions.
+  CompileJsToWasmWrappers(isolate_, compiled_module, counters());
+
+  Handle<WasmModuleObject> result =
+      WasmModuleObject::New(isolate_, compiled_module);
+
   // If we created a wasm script, finish it now and make it public to the
   // debugger.
   if (asm_js_script.is_null()) {
@@ -1575,9 +1581,7 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
     isolate_->debug()->OnAfterCompile(script);
   }
 
-  // Compile JS->wasm wrappers for exported functions.
-  CompileJsToWasmWrappers(isolate_, compiled_module, counters());
-  return WasmModuleObject::New(isolate_, compiled_module);
+  return result;
 }
 
 InstanceBuilder::InstanceBuilder(
@@ -3325,6 +3329,8 @@ class AsyncCompileJob::CompileWrappers : public CompileStep {
   // and the wrappers for the function table elements.
   void RunInForeground() override {
     TRACE_COMPILE("(6) Compile wrappers...\n");
+    // TODO(6792): No longer needed once WebAssembly code is off heap.
+    CodeSpaceMemoryModificationScope modification_scope(job_->isolate_->heap());
     // Compile JS->wasm wrappers for exported functions.
     CompileJsToWasmWrappers(job_->isolate_, job_->compiled_module_,
                             job_->counters());

src/wasm/wasm-code-specialization.cc

@@ -121,9 +121,6 @@ bool CodeSpecialization::ApplyToWholeInstance(
     changed |= ApplyToWasmCode(wasm_function, icache_flush_mode);
   }
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(instance->GetHeap());
-
   // Patch all exported functions (JS_TO_WASM_FUNCTION).
   int reloc_mode = 0;
   // We need to patch WASM_CONTEXT_REFERENCE to put the correct address.
@@ -192,9 +189,6 @@ bool CodeSpecialization::ApplyToWasmCode(Code* code,
   base::Optional<PatchDirectCallsHelper> patch_direct_calls_helper;
   bool changed = false;
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(code->GetHeap());
-
   for (RelocIterator it(code, reloc_mode); !it.done(); it.next()) {
     RelocInfo::Mode mode = it.rinfo()->rmode();
     switch (mode) {

src/wasm/wasm-debug.cc

@@ -672,6 +672,8 @@ void WasmDebugInfo::RedirectToInterpreter(Handle<WasmDebugInfo> debug_info,
   Handle<WasmInstanceObject> instance(debug_info->wasm_instance(), isolate);
   Handle<FixedArray> code_table = instance->compiled_module()->code_table();
   CodeRelocationMap code_to_relocate(isolate->heap());
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   for (int func_index : func_indexes) {
     DCHECK_LE(0, func_index);
     DCHECK_GT(debug_info->wasm_instance()->module()->functions.size(),
@@ -688,8 +690,6 @@ void WasmDebugInfo::RedirectToInterpreter(Handle<WasmDebugInfo> debug_info,
     DCHECK_NULL(code_to_relocate.Find(old_code));
     code_to_relocate.Set(old_code, new_code);
   }
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   RedirectCallsitesInInstance(isolate, *instance, code_to_relocate);
 }
 

src/wasm/wasm-objects.cc

@@ -226,6 +226,8 @@ Handle<FixedArray> WasmTableObject::AddDispatchTable(
 }
 
 void WasmTableObject::Grow(Isolate* isolate, uint32_t count) {
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   Handle<FixedArray> dispatch_tables(this->dispatch_tables());
   DCHECK_EQ(0, dispatch_tables->length() % 4);
   uint32_t old_size = functions()->length();
@@ -306,6 +308,8 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
     code = compiler::CompileWasmToWasmWrapper(
         isolate, exported_function->GetWasmCode(), wasm_function->sig,
         func_index, new_context_address);
+    // TODO(6792): No longer needed once WebAssembly code is off heap.
+    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
     compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
   }
 
@@ -1063,6 +1067,8 @@ void WasmCompiledModule::Reset(Isolate* isolate,
       }
     }
 
+    // TODO(6792): No longer needed once WebAssembly code is off heap.
+    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
     FixedArray* functions = FixedArray::cast(fct_obj);
     for (int i = compiled_module->num_imported_functions(),
              end = functions->length();

test/cctest/heap/test-heap.cc

@@ -200,7 +200,11 @@ HEAP_TEST(TestNewSpaceRefsInCopiedCode) {
   Handle<Code> code =
       isolate->factory()->NewCode(desc, Code::STUB, Handle<Code>());
 
-  Handle<Code> copy = factory->CopyCode(code);
+  Handle<Code> copy;
+  {
+    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
+    copy = factory->CopyCode(code);
+  }
 
   CheckEmbeddedObjectsAreEqual(code, copy);
   CcTest::CollectAllAvailableGarbage();

test/cctest/wasm/wasm-run-utils.cc

@@ -95,6 +95,8 @@ uint32_t TestingModuleBuilder::AddJsFunction(
       *v8::Local<v8::Function>::Cast(CompileRun(source))));
   uint32_t index = AddFunction(sig, Handle<Code>::null(), nullptr);
   js_imports_table->set(0, *isolate_->native_context());
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate_->heap());
   Handle<Code> code = compiler::CompileWasmToJSWrapper(
       isolate_, jsfunc, sig, index, test_module_.origin(), js_imports_table);
   function_code_[index] = code;