Reland "[wasm] Simplify module creation"

Original CL:
> [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}

R=ahaas@chromium.org, clemensh@chromium.org
CC=binji@chromium.org

Change-Id: I03aa901a1df65af28f864d9aabe2b134ea132e99
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1724213
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62996}

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);
@@ -1369,30 +1369,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,
-      wasm::NativeModule::kCanAllocateMoreMemory, std::move(module));
+      isolate, enabled, 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.
@@ -1516,11 +1508,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,
-      wasm::NativeModule::kCanAllocateMoreMemory, std::move(module));
+      isolate_, enabled_features_, std::move(module));
   native_module_->SetWireBytes({std::move(bytes_copy_), wire_bytes_.length()});
   native_module_->SetRuntimeStubs(isolate_);
 
@@ -1534,10 +1523,8 @@ void AsyncCompileJob::PrepareRuntimeObjects() {
   Handle<Script> script =
       CreateWasmScript(isolate_, wire_bytes_, module->source_map_url);
 
-  size_t code_size_estimate =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module);
-  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      isolate_, native_module_, script, code_size_estimate);
+  Handle<WasmModuleObject> module_object =
+      WasmModuleObject::New(isolate_, native_module_, script);
 
   module_object_ = isolate_->global_handles()->Create(*module_object);
 }
@@ -1568,15 +1555,16 @@ void AsyncCompileJob::FinishCompile() {
   // TODO(bbudge) Allow deserialization without wrapper compilation, so we can
   // just compile wrappers here.
   if (!is_after_deserialization) {
-#ifndef V8_EMBEDDED_BUILTINS
-    CompileJsToWasmWrappers(
-        isolate_, module_object_->native_module()->module(),
-        handle(module_object_->export_wrappers(), isolate_));
-#else
-    Handle<FixedArray> export_wrappers =
-        handle(module_object_->export_wrappers(), isolate_);
+#ifdef V8_EMBEDDED_BUILTINS
+    Handle<FixedArray> export_wrappers;
     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.
@@ -2348,7 +2336,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.
@@ -2359,7 +2349,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());
   }
 }
@@ -2604,7 +2594,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;
   WasmFeatures enabled_features = WasmFeaturesFromIsolate(isolate);
@@ -2648,7 +2641,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 =
-      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(
-          native_module->module());
-
-  Handle<WasmModuleObject> module_object =
-      WasmModuleObject::New(isolate, std::move(native_module), script,
-                            export_wrappers, code_size_estimate);
+  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
+      isolate, std::move(native_module), script, export_wrappers);
   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 =
-      WasmModuleObject::New(isolate, std::move(native_module), script,
-                            export_wrappers, code_size_estimate);
+  Handle<WasmModuleObject> module_object = WasmModuleObject::New(
+      isolate, std::move(native_module), script, export_wrappers);
 
   // 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(isolate, wire_bytes, module->source_map_url);
-  size_t code_size = native_module->committed_code_space();
+  Handle<Script> script = CreateWasmScript(
+      isolate, wire_bytes, native_module->module()->source_map_url);
+  Handle<FixedArray> export_wrappers;
+  CompileJsToWasmWrappers(isolate, native_module->module(), &export_wrappers);
   Handle<WasmModuleObject> module_object = WasmModuleObject::New(
-      isolate, std::move(shared_native_module), script, code_size);
-  CompileJsToWasmWrappers(isolate, native_module->module(),
-                          handle(module_object->export_wrappers(), isolate));
+      isolate, std::move(shared_native_module), script, export_wrappers,
+      native_module->committed_code_space());
   {
     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,
-    std::shared_ptr<const wasm::WasmModule> shared_module,
-    OwnedVector<const uint8_t> wire_bytes, Handle<Script> script,
-    Handle<ByteArray> asm_js_offset_table) {
-  // 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;
+    Isolate* isolate, std::shared_ptr<wasm::NativeModule> native_module,
+    Handle<Script> script) {
+  Handle<FixedArray> export_wrappers = isolate->factory()->NewFixedArray(0);
+  return New(isolate, std::move(native_module), script, export_wrappers);
 }
 
 // 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);
-  Handle<FixedArray> export_wrappers =
-      isolate->factory()->NewFixedArray(num_wrappers, AllocationType::kOld);
+  size_t code_size_estimate =
+      wasm::WasmCodeManager::EstimateNativeModuleCodeSize(module);
   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 =
-      OwnedVector<uint8_t>::Of(wire_bytes_vec);
+  auto shared_native_module = isolate->wasm_engine()->NewNativeModule(
+      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(),
-      std::move(wire_bytes_copy), script, Handle<ByteArray>::null());
+      isolate, std::move(shared_native_module), script, export_wrappers);
   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_, {},
-                            script, Handle<ByteArray>::null());
+      WasmModuleObject::New(isolate_, std::move(native_module), script);
   // 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.