Bug 1459555 - Part 2: Don't take locks in BinSourceRuntimeSuppert::*. (r=arai)
authorEric Faust <efaustbmo@gmail.com>
Tue, 02 Oct 2018 01:16:51 -0700
changeset 494890 ac5bef60b83b7d6e9d9445996305b98efd00e200
parent 494889 1487cec16a9c092cf2affd5ef0ecab41c88fb02d
child 494891 3a74539389613df82b090ef6c098134c89432caa
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;
 }