Bug 1459555 - Part 2: Don't take locks in BinSourceRuntimeSuppert::*. (r=arai)
☠☠ backed out by 63ea63571271 ☠ ☠
authorEric Faust <efaustbmo@gmail.com>
Mon, 01 Oct 2018 20:41:48 -0700
changeset 494876 ccdbc1449e13a2b6a75ff55286f905669499cd27
parent 494875 8f2f2bcd57d2d65a5f4412a9c8867a0296a5b271
child 494877 90505ae78229cc37f024fae810c34bf3cc6456be
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1459555
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1459555 - Part 2: Don't take locks in BinSourceRuntimeSuppert::*. (r=arai)
js/src/frontend/BinSourceRuntimeSupport.h
js/src/frontend/BinToken.cpp
js/src/vm/HelperThreads.cpp
--- a/js/src/frontend/BinSourceRuntimeSupport.h
+++ b/js/src/frontend/BinSourceRuntimeSupport.h
@@ -62,19 +62,24 @@ struct BinaryASTSupport {
             }
             return strncmp(key.start_, lookup.start_, key.byteLen_) == 0;
         }
     };
 
     BinaryASTSupport();
 
     JS::Result<const BinVariant*>  binVariant(JSContext*, const CharSlice);
-    JS::Result<const BinField*> binField(JSContext*, const CharSlice);
     JS::Result<const BinKind*> binKind(JSContext*,  const CharSlice);
 
+    bool ensureBinTablesInitialized(JSContext*);
+
+  private:
+    bool ensureBinKindsInitialized(JSContext*);
+    bool ensureBinVariantsInitialized(JSContext*);
+
   private:
     // A HashMap that can be queried without copies from a CharSlice key.
     // Initialized on first call. Keys are CharSlices into static strings.
     using BinKindMap = js::HashMap<const CharSlice, BinKind, CharSlice, js::SystemAllocPolicy>;
     BinKindMap binKindMap_;
 
     using BinFieldMap = js::HashMap<const CharSlice, BinField, CharSlice, js::SystemAllocPolicy>;
     BinFieldMap binFieldMap_;
--- a/js/src/frontend/BinToken.cpp
+++ b/js/src/frontend/BinToken.cpp
@@ -8,17 +8,17 @@
 
 #include "mozilla/Maybe.h"
 
 #include <sys/types.h>
 
 #include "frontend/BinSourceRuntimeSupport.h"
 #include "frontend/TokenStream.h"
 #include "js/Result.h"
-#include "vm/Runtime.h"
+#include "vm/JSContext.h"
 
 namespace js {
 namespace frontend {
 
 const BinaryASTSupport::CharSlice BINKIND_DESCRIPTIONS[] = {
 #define WITH_VARIANT(_, SPEC_NAME) BinaryASTSupport::CharSlice(SPEC_NAME, sizeof(SPEC_NAME) - 1),
     FOR_EACH_BIN_KIND(WITH_VARIANT)
 #undef WITH_VARIANT
@@ -73,56 +73,98 @@ const char* describeBinVariant(const Bin
 
 BinaryASTSupport::BinaryASTSupport()
   : binKindMap_(frontend::BINKIND_LIMIT)
   , binFieldMap_(frontend::BINFIELD_LIMIT)
   , binVariantMap_(frontend::BINVARIANT_LIMIT)
 {
 }
 
-JS::Result<const js::frontend::BinKind*>
-BinaryASTSupport::binKind(JSContext* cx, const CharSlice key)
+/**
+ * It is expected that all bin tables are initialized on the main thread, and that
+ * any helper threads will find the read-only tables properly initialized, so that
+ * they can do their accesses safely without taking any locks.
+ */
+bool
+BinaryASTSupport::ensureBinTablesInitialized(JSContext* cx)
 {
+    return ensureBinKindsInitialized(cx) && ensureBinVariantsInitialized(cx);
+}
+
+bool
+BinaryASTSupport::ensureBinKindsInitialized(JSContext* cx)
+{
+    MOZ_ASSERT(!cx->helperThread());
     if (binKindMap_.empty()) {
         for (size_t i = 0; i < frontend::BINKIND_LIMIT; ++i) {
             const BinKind variant = static_cast<BinKind>(i);
             const CharSlice& key = getBinKind(variant);
             auto ptr = binKindMap_.lookupForAdd(key);
             MOZ_ASSERT(!ptr);
             if (!binKindMap_.add(ptr, key, variant)) {
-                return ReportOutOfMemoryResult(cx);
+                ReportOutOfMemory(cx);
+                return false;
             }
         }
     }
 
-    auto ptr = binKindMap_.lookup(key);
+    return true;
+}
+
+bool
+BinaryASTSupport::ensureBinVariantsInitialized(JSContext* cx)
+{
+    MOZ_ASSERT(!cx->helperThread());
+    if (binVariantMap_.empty()) {
+        for (size_t i = 0; i < frontend::BINVARIANT_LIMIT; ++i) {
+            const BinVariant variant = static_cast<BinVariant>(i);
+            const CharSlice& key = getBinVariant(variant);
+            auto ptr = binVariantMap_.lookupForAdd(key);
+            MOZ_ASSERT(!ptr);
+            if (!binVariantMap_.add(ptr, key, variant)) {
+                ReportOutOfMemory(cx);
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
+
+JS::Result<const js::frontend::BinKind*>
+BinaryASTSupport::binKind(JSContext* cx, const CharSlice key)
+{
+    MOZ_ASSERT_IF(cx->helperThread(), !binKindMap_.empty());
+    if (!cx->helperThread()) {
+        // Initialize Lazily if on main thread.
+        if (!ensureBinKindsInitialized(cx)) {
+            return cx->alreadyReportedError();
+        }
+    }
+
+    auto ptr = binKindMap_.readonlyThreadsafeLookup(key);
     if (!ptr) {
         return nullptr;
     }
 
     return &ptr->value();
 }
 
 JS::Result<const js::frontend::BinVariant*>
 BinaryASTSupport::binVariant(JSContext* cx, const CharSlice key)
 {
-    if (binVariantMap_.empty()) {
-        for (size_t i = 0; i < frontend::BINVARIANT_LIMIT; ++i) {
-            const BinVariant variant = static_cast<BinVariant>(i);
-            const CharSlice& key = getBinVariant(variant);
-            auto ptr = binVariantMap_.lookupForAdd(key);
-            MOZ_ASSERT(!ptr);
-            if (!binVariantMap_.add(ptr, key, variant)) {
-                return ReportOutOfMemoryResult(cx);
-            }
+    MOZ_ASSERT_IF(cx->helperThread(), !binVariantMap_.empty());
+    if (!cx->helperThread()) {
+        // Initialize lazily if on main thread.
+        if (!ensureBinVariantsInitialized(cx)) {
+            return cx->alreadyReportedError();
         }
     }
 
-
-    auto ptr = binVariantMap_.lookup(key);
+    auto ptr = binVariantMap_.readonlyThreadsafeLookup(key);
     if (!ptr) {
         return nullptr;
     }
 
     return &ptr->value();
 }
 
 } // namespace js
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -925,16 +925,19 @@ js::StartOffThreadDecodeMultiScripts(JSC
 
 #if defined(JS_BUILD_BINAST)
 
 bool
 js::StartOffThreadDecodeBinAST(JSContext* cx, const ReadOnlyCompileOptions& options,
                                const uint8_t* buf, size_t length,
                                JS::OffThreadCompileCallback callback, void *callbackData)
 {
+    if (!cx->runtime()->binast().ensureBinTablesInitialized(cx))
+        return false;
+
     auto task = cx->make_unique<BinASTDecodeTask>(cx, buf, length, callback, callbackData);
     if (!task || !StartOffThreadParseTask(cx, task.get(), options)) {
         return false;
     }
 
     Unused << task.release();
     return true;
 }