WebAssembly: don't eagerly checksum
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Dec 2017 22:50:32 +0000 (22:50 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Dec 2017 22:50:32 +0000 (22:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180441
<rdar://problem/35156628>

Reviewed by Saam Barati.

JSTests:

Checksum is now disabled, so tests only have <?> as the module
name.

* wasm/function-tests/nameSection.js:
* wasm/function-tests/stack-overflow.js:
(import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.assertOverflows):
(assertOverflows.assertThrows):
(assertOverflows):
* wasm/function-tests/stack-trace.js:

Source/JavaScriptCore:

Make checksumming of module optional for now. The bots think the
checksum hurt compile-time. I'd measured it and couldn't see a
difference, and still can't at this point in time, but we'll see
if disabling it fixes the bots. If so then I can make it lazy upon
first backtrace construction, or I can try out MD5 instead of
SHA1.

* runtime/Options.h:
* wasm/WasmModuleInformation.cpp:
(JSC::Wasm::ModuleInformation::ModuleInformation):
* wasm/WasmModuleInformation.h:
* wasm/WasmNameSection.h:
(JSC::Wasm::NameSection::NameSection):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225550 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/wasm/function-tests/nameSection.js
JSTests/wasm/function-tests/stack-overflow.js
JSTests/wasm/function-tests/stack-trace.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/Options.h
Source/JavaScriptCore/wasm/WasmModuleInformation.cpp
Source/JavaScriptCore/wasm/WasmModuleInformation.h
Source/JavaScriptCore/wasm/WasmNameSection.h

index 7182b6a..1236efc 100644 (file)
@@ -1,3 +1,21 @@
+2017-12-05  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: don't eagerly checksum
+        https://bugs.webkit.org/show_bug.cgi?id=180441
+        <rdar://problem/35156628>
+
+        Reviewed by Saam Barati.
+
+        Checksum is now disabled, so tests only have <?> as the module
+        name.
+
+        * wasm/function-tests/nameSection.js:
+        * wasm/function-tests/stack-overflow.js:
+        (import.Builder.from.string_appeared_here.import.as.assert.from.string_appeared_here.assertOverflows):
+        (assertOverflows.assertThrows):
+        (assertOverflows):
+        * wasm/function-tests/stack-trace.js:
+
 2017-12-04  JF Bastien  <jfbastien@apple.com>
 
         Proxy all functions, except the $ objects
index f4d28a8..d4dcd35 100644 (file)
@@ -65,8 +65,8 @@ assert.truthy(stacktrace);
 stacktrace = stacktrace.split("\n");
 assert.falsy(stacktrace[0].indexOf("_silly") === -1);
 assert.eq(stacktrace[1], "wasm-stub@[wasm code]"); // the wasm->js stub
-assert.eq(stacktrace[2], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_eggs]@[wasm code]");
-assert.eq(stacktrace[3], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_bacon]@[wasm code]");
-assert.eq(stacktrace[4], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_spam]@[wasm code]");
-assert.eq(stacktrace[5], "C74B341C862F831F4F75068DF4E42AB36FB3446F.wasm-function[_parrot]@[wasm code]");
+assert.eq(stacktrace[2], "<?>.wasm-function[_eggs]@[wasm code]");
+assert.eq(stacktrace[3], "<?>.wasm-function[_bacon]@[wasm code]");
+assert.eq(stacktrace[4], "<?>.wasm-function[_spam]@[wasm code]");
+assert.eq(stacktrace[5], "<?>.wasm-function[_parrot]@[wasm code]");
 assert.eq(stacktrace[6], "wasm-stub@[wasm code]"); // wasm entry
index 2925f97..cfbb5ee 100644 (file)
@@ -47,7 +47,7 @@ import * as assert from '../assert.js'
         assert.truthy(stack.length > 50);
         for (let i = 0; i < 50; ++i) {
             let item = stack[stack.length - i - 1];
-            assert.matches(item, /[A-F0-9]{40}\.wasm-function\[0\]@\[wasm code\]/);
+            assert.eq(item, '<?>.wasm-function[0]@[wasm code]');
         } 
     }
     assertOverflows(i1);
@@ -126,17 +126,17 @@ import * as assert from '../assert.js'
 
         stack = stack.split("\n");
         assert.truthy(stack.length > 50);
-        const oneRe = /[A-F0-9]{40}\.wasm-function\[1\]@\[wasm code\]/;
-        const zeroRe = /[A-F0-9]{40}\.wasm-function\[0\]@\[wasm code\]/;
-        let currentIndex = oneRe.test(stack[stack.length - 1]) ? 1 : 0;
+        const one = '<?>.wasm-function[1]@[wasm code]';
+        const zero = '<?>.wasm-function[0]@[wasm code]';
+        let currentIndex = (one === stack[stack.length - 1]) ? 1 : 0;
         for (let i = 0; i < 50; ++i) {
             let item = stack[stack.length - 1 - i];
             if (currentIndex === 1) {
-                assert.matches(item, oneRe);
+                assert.eq(item, one);
                 currentIndex = 0;
             } else {
                 assert.eq(currentIndex, 0);
-                assert.matches(item, zeroRe);
+                assert.eq(item, zero);
                 currentIndex = 1;
             }
         }
index 2147a37..76e849d 100644 (file)
@@ -43,10 +43,10 @@ for (let i = 0; i < 10000; ++i) {
     stacktrace = stacktrace.split("\n");
     assert.truthy(stacktrace[0].indexOf("imp") !== -1); // the arrow function import named "imp".
     assert.eq(stacktrace[1], "wasm-stub@[wasm code]"); // the wasm->js stub
-    assert.eq(stacktrace[2], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[4]@[wasm code]");
-    assert.eq(stacktrace[3], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[2]@[wasm code]");
-    assert.eq(stacktrace[4], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[3]@[wasm code]");
-    assert.eq(stacktrace[5], "2C77E97D775063A868EF4437DA260245AFA94583.wasm-function[1]@[wasm code]");
+    assert.eq(stacktrace[2], "<?>.wasm-function[4]@[wasm code]");
+    assert.eq(stacktrace[3], "<?>.wasm-function[2]@[wasm code]");
+    assert.eq(stacktrace[4], "<?>.wasm-function[3]@[wasm code]");
+    assert.eq(stacktrace[5], "<?>.wasm-function[1]@[wasm code]");
     assert.eq(stacktrace[6], "wasm-stub@[wasm code]"); // wasm entry
 
     stacktrace = null;
index da8bc01..0fc93e9 100644 (file)
@@ -1,3 +1,25 @@
+2017-12-05  JF Bastien  <jfbastien@apple.com>
+
+        WebAssembly: don't eagerly checksum
+        https://bugs.webkit.org/show_bug.cgi?id=180441
+        <rdar://problem/35156628>
+
+        Reviewed by Saam Barati.
+
+        Make checksumming of module optional for now. The bots think the
+        checksum hurt compile-time. I'd measured it and couldn't see a
+        difference, and still can't at this point in time, but we'll see
+        if disabling it fixes the bots. If so then I can make it lazy upon
+        first backtrace construction, or I can try out MD5 instead of
+        SHA1.
+
+        * runtime/Options.h:
+        * wasm/WasmModuleInformation.cpp:
+        (JSC::Wasm::ModuleInformation::ModuleInformation):
+        * wasm/WasmModuleInformation.h:
+        * wasm/WasmNameSection.h:
+        (JSC::Wasm::NameSection::NameSection):
+
 2017-12-05  Filip Pizlo  <fpizlo@apple.com>
 
         IsoAlignedMemoryAllocator needs to free all of its memory when the VM destructs
index b207799..00c1049 100644 (file)
@@ -478,6 +478,7 @@ constexpr bool enableAsyncIteration = false;
     v(unsigned, maxNumWebAssemblyFastMemories, 4, Normal, nullptr) \
     v(bool, useFastTLSForWasmContext, true, Normal, "If true, we will store context in fast TLS. If false, we will pin it to a register.") \
     v(bool, useCallICsForWebAssemblyToJSCalls, true, Normal, "If true, we will use CallLinkInfo to inline cache Wasm to JS calls.") \
+    v(bool, useEagerWebAssemblyModuleHashing, false, Normal, "Unnamed WebAssembly modules are identified in backtraces through their hash, if available.") \
     v(bool, useObjectRestSpread, true, Normal, "If true, we will enable Object Rest/Spread feature.") \
     v(bool, useArrayAllocationProfiling, true, Normal, "If true, we will use our normal array allocation profiling. If false, the allocation profile will always claim to be undecided.")\
     v(bool, forcePolyProto, false, Normal, "If true, create_this will always create an object with a poly proto structure.")
index 0c80308..cb8db0e 100644 (file)
@@ -44,7 +44,7 @@ namespace JSC { namespace Wasm {
 
 ModuleInformation::ModuleInformation(Vector<uint8_t>&& sourceBytes)
     : source(WTFMove(sourceBytes))
-    , hash(sha1(source))
+    , hash(Options::useEagerWebAssemblyModuleHashing() ? std::make_optional(sha1(source)) : std::nullopt)
     , nameSection(new NameSection(hash))
 {
 }
index c4f5ec8..0bab57e 100644 (file)
@@ -29,6 +29,8 @@
 
 #include "WasmFormat.h"
 
+#include <wtf/Optional.h>
+
 namespace JSC { namespace Wasm {
 
 struct ModuleInformation : public ThreadSafeRefCounted<ModuleInformation> {
@@ -57,7 +59,7 @@ struct ModuleInformation : public ThreadSafeRefCounted<ModuleInformation> {
     uint32_t internalFunctionCount() const { return internalFunctionSignatureIndices.size(); }
 
     const Vector<uint8_t> source;
-    const CString hash;
+    const std::optional<CString> hash;
 
     Vector<Import> imports;
     Vector<SignatureIndex> importFunctionSignatureIndices;
index 457057b..7c02f2f 100644 (file)
@@ -38,11 +38,17 @@ struct NameSection : public ThreadSafeRefCounted<NameSection> {
     WTF_MAKE_NONCOPYABLE(NameSection);
 
 public:
-    NameSection(const CString &hash)
-        : moduleHash(hash.length())
+    NameSection(const std::optional<CString> &hash)
+        : moduleHash(hash ? hash->length() : 3)
     {
-        for (size_t i = 0; i < hash.length(); ++i)
-            moduleHash[i] = static_cast<uint8_t>(*(hash.data() + i));
+        if (hash) {
+            for (size_t i = 0; i < hash->length(); ++i)
+                moduleHash[i] = static_cast<uint8_t>(*(hash->data() + i));
+        } else {
+            moduleHash[0] = '<';
+            moduleHash[1] = '?';
+            moduleHash[2] = '>';
+        }
     }
 
     std::pair<const Name*, RefPtr<NameSection>> get(size_t functionIndexSpace)