Revert "[wasm] Share export wrappers across modules"

This reverts commit b88e7d21.

Reason for revert: Speculative revert for:
https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Linux/7243

Original change's description:
> [wasm] Share export wrappers across modules
>
> Add a map in {IsolateInfo} to share export wrappers across modules. Each
> entry is a weak handle which uses the finalizer to remove itself from
> the map after the last strong reference dies.
>
> R=​clemensb@chromium.org
>
> Bug: chromium:862123
> Change-Id: I1f3a6af6aa4c4e42abfe587354ca14f9da916d91
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2448465
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70348}

TBR=ulan@chromium.org,clemensb@chromium.org,thibaudm@chromium.org

Change-Id: I5ccf4830301939b2b5d7a0d0d769ab9b90536619
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:862123
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2453730Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70358}

src/wasm/function-compiler.cc

@@ -301,45 +301,30 @@ JSToWasmWrapperCompilationUnit::JSToWasmWrapperCompilationUnit(
       job_(use_generic_wrapper_ ? nullptr
                                 : compiler::NewJSToWasmCompilationJob(
                                       isolate, wasm_engine, sig, module,
-                                      is_import, enabled_features)) {
-  JSToWasmWrapperKey key{is_import, *sig};
-  shared_wrapper_ = wasm_engine->GetSharedJSToWasmWrapper(isolate, key);
-  if (!shared_wrapper_.is_null()) {
-    job_ = nullptr;
-    // Make it global to keep it alive until we {Finalize}.
-    shared_wrapper_ = isolate->global_handles()->Create(*shared_wrapper_);
-    GlobalHandles::AnnotateStrongRetainer(
-        shared_wrapper_.location(),
-        "JSToWasmWrapperCompilationUnit::shared_wrapper_");
-  }
-}
+                                      is_import, enabled_features)) {}
 
 JSToWasmWrapperCompilationUnit::~JSToWasmWrapperCompilationUnit() = default;
 
 void JSToWasmWrapperCompilationUnit::Execute() {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
                "wasm.CompileJSToWasmWrapper");
-  if (!use_generic_wrapper_ && shared_wrapper_.is_null()) {
+  if (!use_generic_wrapper_) {
     CompilationJob::Status status = job_->ExecuteJob(nullptr);
     CHECK_EQ(status, CompilationJob::SUCCEEDED);
   }
 }
 
 Handle<Code> JSToWasmWrapperCompilationUnit::Finalize(Isolate* isolate) {
+  Handle<Code> code;
   if (use_generic_wrapper_) {
-    return isolate->builtins()->builtin_handle(
-        Builtins::kGenericJSToWasmWrapper);
-  } else if (!shared_wrapper_.is_null()) {
-    auto code = Handle<Code>::New(*shared_wrapper_, isolate);
-    GlobalHandles::Destroy(shared_wrapper_.location());
-    return code;
+    code =
+        isolate->builtins()->builtin_handle(Builtins::kGenericJSToWasmWrapper);
+  } else {
+    CompilationJob::Status status = job_->FinalizeJob(isolate);
+    CHECK_EQ(status, CompilationJob::SUCCEEDED);
+    code = job_->compilation_info()->code();
   }
-  CompilationJob::Status status = job_->FinalizeJob(isolate);
-  CHECK_EQ(status, CompilationJob::SUCCEEDED);
-  Handle<Code> code = job_->compilation_info()->code();
-  JSToWasmWrapperKey key{is_import_, *sig_};
-  isolate->wasm_engine()->AddSharedJSToWasmWrapper(isolate, key, code);
-  if (must_record_function_compilation(isolate)) {
+  if (!use_generic_wrapper_ && must_record_function_compilation(isolate)) {
     RecordWasmHeapStubCompilation(
         isolate, code, "%s", job_->compilation_info()->GetDebugName().get());
   }

src/wasm/function-compiler.h

@@ -11,7 +11,6 @@
 #include "src/trap-handler/trap-handler.h"
 #include "src/wasm/compilation-environment.h"
 #include "src/wasm/function-body-decoder.h"
-#include "src/wasm/wasm-engine.h"
 #include "src/wasm/wasm-limits.h"
 #include "src/wasm/wasm-module.h"
 #include "src/wasm/wasm-tier.h"
@@ -136,7 +135,6 @@ class V8_EXPORT_PRIVATE JSToWasmWrapperCompilationUnit final {
   bool is_import_;
   const FunctionSig* sig_;
   bool use_generic_wrapper_;
-  Handle<Code> shared_wrapper_;
   std::unique_ptr<OptimizedCompilationJob> job_;
 };
 

src/wasm/module-compiler.cc

@@ -1380,21 +1380,24 @@ CompilationExecutionResult ExecuteCompilationUnits(
   return kNoMoreUnits;
 }
 
+using JSToWasmWrapperKey = std::pair<bool, FunctionSig>;
+
 // Returns the number of units added.
 int AddExportWrapperUnits(Isolate* isolate, WasmEngine* wasm_engine,
                           NativeModule* native_module,
                           CompilationUnitBuilder* builder,
                           const WasmFeatures& enabled_features) {
-  std::unordered_set<JSToWasmWrapperKey, JSToWasmWrapperKeyHash> keys;
+  std::unordered_set<JSToWasmWrapperKey, base::hash<JSToWasmWrapperKey>> keys;
   for (auto exp : native_module->module()->export_table) {
     if (exp.kind != kExternalFunction) continue;
     auto& function = native_module->module()->functions[exp.index];
-    JSToWasmWrapperKey key{function.imported, *function.sig};
-    if (!keys.insert(key).second) continue;
-    auto unit = std::make_shared<JSToWasmWrapperCompilationUnit>(
-        isolate, wasm_engine, function.sig, native_module->module(),
-        function.imported, enabled_features);
-    builder->AddJSToWasmWrapperUnit(std::move(unit));
+    JSToWasmWrapperKey key(function.imported, *function.sig);
+    if (keys.insert(key).second) {
+      auto unit = std::make_shared<JSToWasmWrapperCompilationUnit>(
+          isolate, wasm_engine, function.sig, native_module->module(),
+          function.imported, enabled_features);
+      builder->AddJSToWasmWrapperUnit(std::move(unit));
+    }
   }
 
   return static_cast<int>(keys.size());
@@ -3310,11 +3313,11 @@ void CompilationStateImpl::WaitForCompilationEvent(
 
 namespace {
 using JSToWasmWrapperQueue =
-    WrapperQueue<JSToWasmWrapperKey, JSToWasmWrapperKeyHash>;
+    WrapperQueue<JSToWasmWrapperKey, base::hash<JSToWasmWrapperKey>>;
 using JSToWasmWrapperUnitMap =
     std::unordered_map<JSToWasmWrapperKey,
                        std::unique_ptr<JSToWasmWrapperCompilationUnit>,
-                       JSToWasmWrapperKeyHash>;
+                       base::hash<JSToWasmWrapperKey>>;
 
 class CompileJSToWasmWrapperJob final : public JobTask {
  public:
@@ -3364,7 +3367,7 @@ void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module,
   for (auto exp : module->export_table) {
     if (exp.kind != kExternalFunction) continue;
     auto& function = module->functions[exp.index];
-    JSToWasmWrapperKey key{function.imported, *function.sig};
+    JSToWasmWrapperKey key(function.imported, *function.sig);
     if (queue.insert(key)) {
       auto unit = std::make_unique<JSToWasmWrapperCompilationUnit>(
           isolate, isolate->wasm_engine(), function.sig, module,
@@ -3394,7 +3397,7 @@ void CompileJsToWasmWrappers(Isolate* isolate, const WasmModule* module,
     JSToWasmWrapperKey key = pair.first;
     JSToWasmWrapperCompilationUnit* unit = pair.second.get();
     Handle<Code> code = unit->Finalize(isolate);
-    int wrapper_index = GetExportWrapperIndex(module, &key.sig, key.is_import);
+    int wrapper_index = GetExportWrapperIndex(module, &key.second, key.first);
     (*export_wrappers_out)->set(wrapper_index, *code);
     RecordStats(*code, isolate->counters());
   }

src/wasm/wasm-engine.cc

@@ -22,7 +22,6 @@
 #include "src/wasm/module-decoder.h"
 #include "src/wasm/module-instantiate.h"
 #include "src/wasm/streaming-decoder.h"
-#include "src/wasm/value-type.h"
 #include "src/wasm/wasm-debug.h"
 #include "src/wasm/wasm-limits.h"
 #include "src/wasm/wasm-objects-inl.h"
@@ -154,37 +153,6 @@ class WeakScriptHandle {
   std::unique_ptr<Address*> location_;
 };
 
-void WeakJSToWasmWrapperFinalizer(const v8::WeakCallbackInfo<void>& data) {
-  Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
-  JSToWasmWrapperKey* key =
-      reinterpret_cast<JSToWasmWrapperKey*>(data.GetParameter());
-  isolate->wasm_engine()->EraseSharedJSToWasmWrapper(isolate, *key);
-}
-
-class WeakJSToWasmWrapperHandle {
- public:
-  explicit WeakJSToWasmWrapperHandle(const JSToWasmWrapperKey& key,
-                                     Handle<Code> handle)
-      : key_(key) {
-    Handle<Code> global_handle =
-        handle->GetIsolate()->global_handles()->Create(*handle);
-    location_ = global_handle.location();
-    GlobalHandles::MakeWeak(location_, &key_, WeakJSToWasmWrapperFinalizer,
-                            v8::WeakCallbackType::kParameter);
-  }
-
-  // Invoked when we erase the entry in the finalizer.
-  ~WeakJSToWasmWrapperHandle() { GlobalHandles::Destroy(location_); }
-
-  Handle<Code> handle() { return Handle<Code>(location_); }
-
- private:
-  JSToWasmWrapperKey key_;
-  Address* location_;
-
-  DISALLOW_COPY_AND_ASSIGN(WeakJSToWasmWrapperHandle);
-};
-
 }  // namespace
 
 std::shared_ptr<NativeModule> NativeModuleCache::MaybeGetNativeModule(
@@ -392,12 +360,6 @@ struct WasmEngine::IsolateInfo {
 
   // Keep new modules in tiered down state.
   bool keep_tiered_down = false;
-
-  using SharedExportWrappers =
-      std::unordered_map<OwnedJSToWasmWrapperKey,
-                         std::unique_ptr<WeakJSToWasmWrapperHandle>,
-                         OwnedJSToWasmWrapperKeyHash>;
-  SharedExportWrappers export_wrappers;
 };
 
 struct WasmEngine::NativeModuleInfo {
@@ -1483,86 +1445,6 @@ std::shared_ptr<WasmEngine> WasmEngine::GetWasmEngine() {
   return *GetSharedWasmEngine();
 }
 
-namespace {
-bool IsSignatureModuleDependent(const FunctionSig& sig) {
-  for (auto& type : sig.all()) {
-    if (type != kWasmI32 && type != kWasmI64 && type != kWasmF32 &&
-        type != kWasmF64) {
-      return true;
-    }
-  }
-  return false;
-}
-}  // namespace
-
-Handle<Code> WasmEngine::GetSharedJSToWasmWrapper(
-    Isolate* isolate, const JSToWasmWrapperKey& key) {
-  if (IsSignatureModuleDependent(key.sig)) return Handle<Code>();
-  IsolateInfo::SharedExportWrappers* export_wrappers;
-  {
-    base::MutexGuard guard(&mutex_);
-    auto it = isolates_.find(isolate);
-    DCHECK_NE(isolates_.end(), it);
-    export_wrappers = &it->second->export_wrappers;
-  }
-  auto wrapper_it = export_wrappers->find({key, nullptr});
-  if (wrapper_it != export_wrappers->end()) {
-    Handle<Code> weak_handle = wrapper_it->second->handle();
-    if (weak_handle.is_null()) {
-      export_wrappers->erase(wrapper_it);
-    } else {
-      return Handle<Code>::New(*weak_handle, isolate);
-    }
-  }
-  return Handle<Code>();
-}
-
-Handle<Code> WasmEngine::AddSharedJSToWasmWrapper(Isolate* isolate,
-                                                  const JSToWasmWrapperKey& key,
-                                                  Handle<Code> code) {
-  if (IsSignatureModuleDependent(key.sig)) return code;
-  IsolateInfo::SharedExportWrappers* export_wrappers;
-  {
-    base::MutexGuard guard(&mutex_);
-    auto it = isolates_.find(isolate);
-    DCHECK_NE(isolates_.end(), it);
-    export_wrappers = &it->second->export_wrappers;
-  }
-  auto wrapper_it = export_wrappers->find({key, nullptr});
-  if (wrapper_it == export_wrappers->end()) {
-    size_t num_types = key.sig.parameter_count() + key.sig.return_count();
-    auto owned_types = std::make_unique<ValueType[]>(num_types);
-    std::copy(key.sig.all().begin(), key.sig.all().end(), owned_types.get());
-    FunctionSig sig(key.sig.return_count(), key.sig.parameter_count(),
-                    owned_types.get());
-    OwnedJSToWasmWrapperKey owned_key{{key.is_import, sig},
-                                      std::move(owned_types)};
-    auto p = export_wrappers->emplace(
-        std::move(owned_key),
-        std::make_unique<WeakJSToWasmWrapperHandle>(owned_key.key, code));
-    DCHECK(p.second);
-    return p.first->second->handle();
-  } else {
-    Handle<Code> weak_handle = wrapper_it->second->handle();
-    DCHECK(!weak_handle.is_null());
-    return Handle<Code>::New(*weak_handle, isolate);
-  }
-}
-
-void WasmEngine::EraseSharedJSToWasmWrapper(Isolate* isolate,
-                                            const JSToWasmWrapperKey& key) {
-  IsolateInfo::SharedExportWrappers* export_wrappers;
-  {
-    base::MutexGuard guard(&mutex_);
-    auto it = isolates_.find(isolate);
-    DCHECK_NE(isolates_.end(), it);
-    export_wrappers = &it->second->export_wrappers;
-  }
-  auto wrapper_it = export_wrappers->find({key, nullptr});
-  DCHECK_NE(wrapper_it, export_wrappers->end());
-  export_wrappers->erase(wrapper_it);
-}
-
 // {max_mem_pages} is declared in wasm-limits.h.
 uint32_t max_mem_pages() {
   STATIC_ASSERT(kV8MaxWasmMemoryPages <= kMaxUInt32);

src/wasm/wasm-engine.h

@@ -132,43 +132,6 @@ class NativeModuleCache {
   base::ConditionVariable cache_cv_;
 };
 
-struct JSToWasmWrapperKey {
-  bool is_import;
-  FunctionSig sig;
-  bool operator==(const JSToWasmWrapperKey& other) const {
-    return is_import == other.is_import &&
-           sig.parameter_count() == other.sig.parameter_count() &&
-           sig.return_count() == other.sig.return_count() &&
-           std::equal(sig.all().begin(), sig.all().end(),
-                      other.sig.all().begin(), other.sig.all().end());
-  }
-};
-
-struct JSToWasmWrapperKeyHash {
-  size_t operator()(const JSToWasmWrapperKey& key) const {
-    size_t hash = base::hash_combine(key.is_import, key.sig.parameter_count(),
-                                     key.sig.return_count());
-    for (auto& type : key.sig.all()) {
-      hash = base::hash_combine(hash, type);
-    }
-    return hash;
-  }
-};
-
-struct OwnedJSToWasmWrapperKey {
-  JSToWasmWrapperKey key;
-  std::unique_ptr<ValueType[]> types;
-  bool operator==(const OwnedJSToWasmWrapperKey& other) const {
-    return key == other.key;
-  }
-};
-
-struct OwnedJSToWasmWrapperKeyHash {
-  size_t operator()(const OwnedJSToWasmWrapperKey& owned_key) const {
-    return JSToWasmWrapperKeyHash()(owned_key.key);
-  }
-};
-
 // The central data structure that represents an engine instance capable of
 // loading, instantiating, and executing Wasm code.
 class V8_EXPORT_PRIVATE WasmEngine {
@@ -382,16 +345,6 @@ class V8_EXPORT_PRIVATE WasmEngine {
   // engine lifetime decisions during Isolate bootstrapping.
   static std::shared_ptr<WasmEngine> GetWasmEngine();
 
-  Handle<Code> GetSharedJSToWasmWrapper(Isolate*, const JSToWasmWrapperKey&);
-
-  // Insert the (key, code) pair if it is not in the map already.
-  // Return the code object associated with the key after insertion. This will
-  // be the Handle<Code> argument if the key is new, and the old Handle<Code>
-  // otherwise.
-  Handle<Code> AddSharedJSToWasmWrapper(Isolate*, const JSToWasmWrapperKey&,
-                                        Handle<Code>);
-  void EraseSharedJSToWasmWrapper(Isolate*, const JSToWasmWrapperKey&);
-
  private:
   struct CurrentGCInfo;
   struct IsolateInfo;

test/cctest/BUILD.gn

@@ -310,7 +310,6 @@ v8_source_set("cctest_sources") {
     "wasm/test-wasm-codegen.cc",
     "wasm/test-wasm-debug-evaluate.cc",
     "wasm/test-wasm-debug-evaluate.h",
-    "wasm/test-wasm-export-wrapper-cache.cc",
     "wasm/test-wasm-import-wrapper-cache.cc",
     "wasm/test-wasm-metrics.cc",
     "wasm/test-wasm-serialization.cc",

test/cctest/wasm/test-wasm-export-wrapper-cache.cc

-// Copyright 2020 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "test/cctest/cctest.h"
-#include "test/cctest/wasm/wasm-run-utils.h"
-#include "test/common/wasm/wasm-macro-gen.h"
-
-namespace v8 {
-namespace internal {
-namespace wasm {
-
-namespace {
-
-TEST(RunWasmTurbofan_ExportSameSig) {
-  WasmRunner<int32_t> r1(TestExecutionTier::kTurbofan);
-  BUILD(r1, kExprI32Const, 0);
-
-  WasmRunner<int32_t> r2(TestExecutionTier::kTurbofan);
-  BUILD(r2, kExprI32Const, 1);
-
-  Handle<JSFunction> f1 = r1.builder().WrapCode(0);
-  auto shared = f1->shared();
-  auto wasm_exported_function_data = shared.wasm_exported_function_data();
-  Code code1 = wasm_exported_function_data.wrapper_code();
-
-  Handle<JSFunction> f2 = r2.builder().WrapCode(0);
-  shared = f2->shared();
-  wasm_exported_function_data = shared.wasm_exported_function_data();
-  Code code2 = wasm_exported_function_data.wrapper_code();
-
-  CHECK_EQ(code1, code2);
-}
-
-}  // namespace
-
-}  // namespace wasm
-}  // namespace internal
-}  // namespace v8