[wasm] Simplify module creation

This includes WasmEngine::NewNativeModule() and WasmModuleObject::New().
The intent is to make the various ways of creating a module (sync,
async, deserialize, import) more similar.

After this change, a NativeModule will always be created before a
WasmModuleObject. This will make it easier to look up a cached
NativeModule given its wire bytes.

The following changes are made:

* Use WasmCodeManager::EstimateNativeModuleCodeSize() to find the code
  size estimate by default. A different code size estimate is only used in
  tests.
* Change CompileJsToWasmWrappers() to allocate a new FixedArray instead of
  assuming the array was created with the correct size. This simplifies
  WasmModuleObject::New(), and matches what CompileToNativeModule()
  does.
* Remove the WasmModuleObject::New() constructor that creates a
  NativeModule. This case was only used in DeserializeNativeModule() and
  in test code.

Change-Id: I6bdfc425057f92de11abbbf702d052d40aa8267d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1717497
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62925}

src/wasm/module-compiler.cc

@@ -401,7 +401,7 @@ class CompilationStateImpl {
   std::shared_ptr<JSToWasmWrapperCompilationUnit>
   GetNextJSToWasmWrapperCompilationUnit();
   void FinalizeJSToWasmWrappers(Isolate* isolate, const WasmModule* module,
-                                Handle<FixedArray> export_wrappers);
+                                Handle<FixedArray>* export_wrappers_out);
 
   void OnFinishedUnits(Vector<WasmCode*>);
   void OnFinishedJSToWasmWrapperUnits(int num);
@@ -1365,30 +1365,22 @@ std::shared_ptr<NativeModule> CompileToNativeModule(
   OwnedVector<uint8_t> wire_bytes_copy =
       OwnedVector<uint8_t>::Of(wire_bytes.module_bytes());
 
-  // Create and compile the native module.
-  size_t code_size_estimate =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
-
   // Create a new {NativeModule} first.
   auto native_module = isolate->wasm_engine()->NewNativeModule(
-      isolate, enabled, code_size_estimate,
+      isolate, enabled, std::move(module));
-      wasm::NativeModule::kCanAllocateMoreMemory, std::move(module));
   native_module->SetWireBytes(std::move(wire_bytes_copy));
   native_module->SetRuntimeStubs(isolate);
 
   CompileNativeModule(isolate, thrower, wasm_module, native_module.get());
   if (thrower->error()) return {};
 
-  int num_wrappers = MaxNumExportWrappers(native_module->module());
-  *export_wrappers_out =
-      isolate->factory()->NewFixedArray(num_wrappers, AllocationType::kOld);
 #ifdef V8_EMBEDDED_BUILTINS
   Impl(native_module->compilation_state())
       ->FinalizeJSToWasmWrappers(isolate, native_module->module(),
-                                 *export_wrappers_out);
+                                 export_wrappers_out);
 #else
   CompileJsToWasmWrappers(isolate, native_module->module(),
-                          *export_wrappers_out);
+                          export_wrappers_out);
 #endif
 
   // Log the code within the generated module for profiling.
@@ -1511,11 +1503,8 @@ void AsyncCompileJob::CreateNativeModule(
   // breakpoints on a (potentially empty) subset of the instances.
   // Create the module object.
 
-  size_t code_size_estimate =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
   native_module_ = isolate_->wasm_engine()->NewNativeModule(
-      isolate_, enabled_features_, code_size_estimate,
+      isolate_, enabled_features_, std::move(module));
-      wasm::NativeModule::kCanAllocateMoreMemory, std::move(module));
   native_module_->SetWireBytes({std::move(bytes_copy_), wire_bytes_.length()});
   native_module_->SetRuntimeStubs(isolate_);
 
@@ -1529,10 +1518,8 @@ void AsyncCompileJob::PrepareRuntimeObjects() {
   Handle<Script> script =
       CreateWasmScript(isolate_, wire_bytes_, module->source_map_url);
 
-  size_t code_size_estimate =
+  Handle<WasmModuleObject> module_object =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module);
+      WasmModuleObject::New(isolate_, native_module_, script);
-  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      isolate_, native_module_, script, code_size_estimate);
 
   module_object_ = isolate_->global_handles()->Create(*module_object);
 }
@@ -1560,18 +1547,19 @@ void AsyncCompileJob::FinishCompile() {
 
   auto compilation_state =
       Impl(module_object_->native_module()->compilation_state());
-#ifndef V8_EMBEDDED_BUILTINS
+#ifdef V8_EMBEDDED_BUILTINS
-  CompileJsToWasmWrappers(isolate_, module_object_->native_module()->module(),
-                          handle(module_object_->export_wrappers(), isolate_));
-#else
   // TODO(bbudge) Allow deserialization without wrapper compilation, so we can
   // just compile wrappers here.
   if (!is_after_deserialization) {
-    Handle<FixedArray> export_wrappers =
+    Handle<FixedArray> export_wrappers;
-        handle(module_object_->export_wrappers(), isolate_);
     compilation_state->FinalizeJSToWasmWrappers(
-        isolate_, module_object_->module(), export_wrappers);
+        isolate_, module_object_->module(), &export_wrappers);
+    module_object_->set_export_wrappers(*export_wrappers);
   }
+#else
+  Handle<FixedArray> export_wrappers;
+  CompileJsToWasmWrappers(isolate_, module_object_->module(), &export_wrappers);
+  module_object_->set_export_wrappers(*export_wrappers);
 #endif
   // We can only update the feature counts once the entire compile is done.
   compilation_state->PublishDetectedFeatures(isolate_);
@@ -2340,7 +2328,9 @@ CompilationStateImpl::GetNextJSToWasmWrapperCompilationUnit() {
 
 void CompilationStateImpl::FinalizeJSToWasmWrappers(
     Isolate* isolate, const WasmModule* module,
-    Handle<FixedArray> export_wrappers) {
+    Handle<FixedArray>* export_wrappers_out) {
+  *export_wrappers_out = isolate->factory()->NewFixedArray(
+      MaxNumExportWrappers(module), AllocationType::kOld);
   // TODO(6792): Wrappers below are allocated with {Factory::NewCode}. As an
   // optimization we keep the code space unlocked to avoid repeated unlocking
   // because many such wrapper are allocated in sequence below.
@@ -2351,7 +2341,7 @@ void CompilationStateImpl::FinalizeJSToWasmWrappers(
     Handle<Code> code = unit->Finalize(isolate);
     int wrapper_index =
         GetExportWrapperIndex(module, unit->sig(), unit->is_import());
-    export_wrappers->set(wrapper_index, *code);
+    (*export_wrappers_out)->set(wrapper_index, *code);
     RecordStats(*code, isolate->counters());
   }
 }
@@ -2596,7 +2586,10 @@ class CompileJSToWasmWrapperTask final : public CancelableTask {
 }  // namespace
 
 void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module,
-                             Handle<FixedArray> export_wrappers) {
+                             Handle<FixedArray>* export_wrappers_out) {
+  *export_wrappers_out = isolate->factory()->NewFixedArray(
+      MaxNumExportWrappers(module), AllocationType::kOld);
+
   JSToWasmWrapperQueue queue;
   JSToWasmWrapperUnitMap compilation_units;
 
@@ -2638,7 +2631,7 @@ void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module,
     JSToWasmWrapperCompilationUnit* unit = pair.second.get();
     Handle<Code> code = unit->Finalize(isolate);
     int wrapper_index = GetExportWrapperIndex(module, &key.second, key.first);
-    export_wrappers->set(wrapper_index, *code);
+    (*export_wrappers_out)->set(wrapper_index, *code);
     RecordStats(*code, isolate->counters());
   }
 }

src/wasm/module-compiler.h

@@ -46,7 +46,7 @@ std::shared_ptr<NativeModule> CompileToNativeModule(
 
 V8_EXPORT_PRIVATE
 void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module,
-                             Handle<FixedArray> export_wrappers);
+                             Handle<FixedArray>* export_wrappers_out);
 
 // Compiles the wrapper for this (kind, sig) pair and sets the corresponding
 // cache entry. Assumes the key already exists in the cache but has not been

src/wasm/wasm-engine.cc

@@ -278,13 +278,8 @@ Handle<WasmModuleObject> WasmEngine::FinalizeTranslatedAsmJs(
       asm_wasm_data->managed_native_module().get();
   Handle<FixedArray> export_wrappers =
       handle(asm_wasm_data->export_wrappers(), isolate);
-  size_t code_size_estimate =
+  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(
+      isolate, std::move(native_module), script, export_wrappers);
-          native_module->module());
-
-  Handle<WasmModuleObject> module_object =
-      WasmModuleObject::New(isolate, std::move(native_module), script,
-                            export_wrappers, code_size_estimate);
   module_object->set_asm_js_offset_table(asm_wasm_data->asm_js_offset_table());
   return module_object;
 }
@@ -310,9 +305,6 @@ MaybeHandle<WasmModuleObject> WasmEngine::SyncCompile(
 
   Handle<Script> script =
       CreateWasmScript(isolate, bytes, native_module->module()->source_map_url);
-  size_t code_size_estimate =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(
-          native_module->module());
 
   // Create the module object.
   // TODO(clemensh): For the same module (same bytes / same hash), we should
@@ -323,9 +315,8 @@ MaybeHandle<WasmModuleObject> WasmEngine::SyncCompile(
   // and information needed at instantiation time. This object needs to be
   // serializable. Instantiation may occur off a deserialized version of this
   // object.
-  Handle<WasmModuleObject> module_object =
+  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      WasmModuleObject::New(isolate, std::move(native_module), script,
+      isolate, std::move(native_module), script, export_wrappers);
-                            export_wrappers, code_size_estimate);
 
   // Finish the Wasm script now and make it public to the debugger.
   isolate->debug()->OnAfterCompile(script);
@@ -451,14 +442,13 @@ Handle<WasmModuleObject> WasmEngine::ImportNativeModule(
     Isolate* isolate, std::shared_ptr<NativeModule> shared_native_module) {
   NativeModule* native_module = shared_native_module.get();
   ModuleWireBytes wire_bytes(native_module->wire_bytes());
-  const WasmModule* module = native_module->module();
+  Handle<Script> script = CreateWasmScript(
-  Handle<Script> script =
+      isolate, wire_bytes, native_module->module()->source_map_url);
-      CreateWasmScript(isolate, wire_bytes, module->source_map_url);
+  Handle<FixedArray> export_wrappers;
-  size_t code_size = native_module->committed_code_space();
+  CompileJsToWasmWrappers(isolate, native_module->module(), &export_wrappers);
   Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      isolate, std::move(shared_native_module), script, code_size);
+      isolate, std::move(shared_native_module), script, export_wrappers,
-  CompileJsToWasmWrappers(isolate, native_module->module(),
+      native_module->committed_code_space());
-                          handle(module_object->export_wrappers(), isolate));
   {
     base::MutexGuard lock(&mutex_);
     DCHECK_EQ(1, isolates_.count(isolate));
@@ -680,6 +670,16 @@ void WasmEngine::LogOutstandingCodesForIsolate(Isolate* isolate) {
   WasmCode::DecrementRefCount(VectorOf(code_to_log));
 }
 
+std::shared_ptr<NativeModule> WasmEngine::NewNativeModule(
+    Isolate* isolate, const WasmFeatures& enabled,
+    std::shared_ptr<const WasmModule> module) {
+  size_t code_size_estimate =
+      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module.get());
+  return NewNativeModule(isolate, enabled, code_size_estimate,
+                         wasm::NativeModule::kCanAllocateMoreMemory,
+                         std::move(module));
+}
+
 std::shared_ptr<NativeModule> WasmEngine::NewNativeModule(
     Isolate* isolate, const WasmFeatures& enabled, size_t code_size_estimate,
     bool can_request_more, std::shared_ptr<const WasmModule> module) {

src/wasm/wasm-engine.h

@@ -180,6 +180,9 @@ class V8_EXPORT_PRIVATE WasmEngine {
   // is determined with a heuristic based on the total size of wasm
   // code. The native module may later request more memory.
   // TODO(titzer): isolate is only required here for CompilationState.
+  std::shared_ptr<NativeModule> NewNativeModule(
+      Isolate* isolate, const WasmFeatures& enabled_features,
+      std::shared_ptr<const WasmModule> module);
   std::shared_ptr<NativeModule> NewNativeModule(
       Isolate* isolate, const WasmFeatures& enabled_features,
       size_t code_size_estimate, bool can_request_more,

src/wasm/wasm-objects.cc

@@ -207,36 +207,19 @@ enum DispatchTableElements : int {
 
 // static
 Handle<WasmModuleObject> WasmModuleObject::New(
-    Isolate* isolate, const wasm::WasmFeatures& enabled,
+    Isolate* isolate, std::shared_ptr<wasm::NativeModule> native_module,
-    std::shared_ptr<const wasm::WasmModule> shared_module,
+    Handle<Script> script) {
-    OwnedVector<const uint8_t> wire_bytes, Handle<Script> script,
+  Handle<FixedArray> export_wrappers = isolate->factory()->NewFixedArray(0);
-    Handle<ByteArray> asm_js_offset_table) {
+  return New(isolate, std::move(native_module), script, export_wrappers);
-  // Create a new {NativeModule} first.
-  size_t code_size_estimate =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(shared_module.get());
-  auto native_module = isolate->wasm_engine()->NewNativeModule(
-      isolate, enabled, code_size_estimate,
-      wasm::NativeModule::kCanAllocateMoreMemory, std::move(shared_module));
-  native_module->SetWireBytes(std::move(wire_bytes));
-  native_module->SetRuntimeStubs(isolate);
-
-  // Delegate to the shared {WasmModuleObject::New} allocator.
-  Handle<WasmModuleObject> module_object =
-      New(isolate, std::move(native_module), script, code_size_estimate);
-  if (!asm_js_offset_table.is_null()) {
-    module_object->set_asm_js_offset_table(*asm_js_offset_table);
-  }
-  return module_object;
 }
 
 // static
 Handle<WasmModuleObject> WasmModuleObject::New(
     Isolate* isolate, std::shared_ptr<wasm::NativeModule> native_module,
-    Handle<Script> script, size_t code_size_estimate) {
+    Handle<Script> script, Handle<FixedArray> export_wrappers) {
   const WasmModule* module = native_module->module();
-  int num_wrappers = MaxNumExportWrappers(module);
+  size_t code_size_estimate =
-  Handle<FixedArray> export_wrappers =
+      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module);
-      isolate->factory()->NewFixedArray(num_wrappers, AllocationType::kOld);
   return New(isolate, std::move(native_module), script, export_wrappers,
              code_size_estimate);
 }

src/wasm/wasm-objects.h

@@ -139,18 +139,14 @@ class WasmModuleObject : public JSObject {
   DEFINE_FIELD_OFFSET_CONSTANTS(JSObject::kHeaderSize,
                                 TORQUE_GENERATED_WASM_MODULE_OBJECT_FIELDS)
 
-  // Creates a new {WasmModuleObject} with a new {NativeModule} underneath.
-  V8_EXPORT_PRIVATE static Handle<WasmModuleObject> New(
-      Isolate* isolate, const wasm::WasmFeatures& enabled,
-      std::shared_ptr<const wasm::WasmModule> module,
-      OwnedVector<const uint8_t> wire_bytes, Handle<Script> script,
-      Handle<ByteArray> asm_js_offset_table);
-
   // Creates a new {WasmModuleObject} for an existing {NativeModule} that is
   // reference counted and might be shared between multiple Isolates.
   V8_EXPORT_PRIVATE static Handle<WasmModuleObject> New(
       Isolate* isolate, std::shared_ptr<wasm::NativeModule> native_module,
-      Handle<Script> script, size_t code_size_estimate);
+      Handle<Script> script);
+  V8_EXPORT_PRIVATE static Handle<WasmModuleObject> New(
+      Isolate* isolate, std::shared_ptr<wasm::NativeModule> native_module,
+      Handle<Script> script, Handle<FixedArray> export_wrappers);
   V8_EXPORT_PRIVATE static Handle<WasmModuleObject> New(
       Isolate* isolate, std::shared_ptr<wasm::NativeModule> native_module,
       Handle<Script> script, Handle<FixedArray> export_wrappers,

src/wasm/wasm-serialization.cc

@@ -625,12 +625,17 @@ MaybeHandle<WasmModuleObject> DeserializeNativeModule(
   Handle<Script> script =
       CreateWasmScript(isolate, wire_bytes, module->source_map_url);
 
-  OwnedVector<uint8_t> wire_bytes_copy =
+  auto shared_native_module = isolate->wasm_engine()->NewNativeModule(
-      OwnedVector<uint8_t>::Of(wire_bytes_vec);
+      isolate, enabled_features, std::move(decode_result.value()));
+  shared_native_module->SetWireBytes(OwnedVector<uint8_t>::Of(wire_bytes_vec));
+  shared_native_module->SetRuntimeStubs(isolate);
+
+  Handle<FixedArray> export_wrappers;
+  CompileJsToWasmWrappers(isolate, shared_native_module->module(),
+                          &export_wrappers);
 
   Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      isolate, enabled_features, std::move(decode_result).value(),
+      isolate, std::move(shared_native_module), script, export_wrappers);
-      std::move(wire_bytes_copy), script, Handle<ByteArray>::null());
   NativeModule* native_module = module_object->native_module();
 
   NativeModuleDeserializer deserializer(native_module);
@@ -639,9 +644,6 @@ MaybeHandle<WasmModuleObject> DeserializeNativeModule(
   Reader reader(data + kVersionSize);
   if (!deserializer.Read(&reader)) return {};
 
-  CompileJsToWasmWrappers(isolate, native_module->module(),
-                          handle(module_object->export_wrappers(), isolate));
-
   // Log the code within the generated module for profiling.
   native_module->LogWasmCodes(isolate);
 

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

@@ -324,9 +324,14 @@ Handle<WasmInstanceObject> TestingModuleBuilder::InitInstanceObject() {
   Handle<Script> script =
       isolate_->factory()->NewScript(isolate_->factory()->empty_string());
   script->set_type(Script::TYPE_WASM);
+
+  auto native_module = isolate_->wasm_engine()->NewNativeModule(
+      isolate_, enabled_features_, test_module_);
+  native_module->SetWireBytes(OwnedVector<const uint8_t>());
+  native_module->SetRuntimeStubs(isolate_);
+
   Handle<WasmModuleObject> module_object =
-      WasmModuleObject::New(isolate_, enabled_features_, test_module_, {},
+      WasmModuleObject::New(isolate_, std::move(native_module), script);
-                            script, Handle<ByteArray>::null());
   // This method is called when we initialize TestEnvironment. We don't
   // have a memory yet, so we won't create it here. We'll update the
   // interpreter when we get a memory. We do have globals, though.