Changeset 228966 in webkit

Timestamp:

Feb 23, 2018 3:16:59 PM (6 years ago)

Author:

gskachkov@gmail.com

Message:

WebAssembly: cache memory address / size on instance
​https://bugs.webkit.org/show_bug.cgi?id=177305

Reviewed by JF Bastien.

JSTests:

• wasm/function-tests/memory-reuse.js: Added.

(createWasmInstance):
(doCheckTrap):
(doMemoryGrow):
(doCheck):
(checkWasmInstancesWithSharedMemory):

Source/JavaScriptCore:

Cache memory address/size in wasm:Instance to avoid load wasm:Memory
object during access to memory and memory size property in JiT

• wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::restoreWebAssemblyGlobalState):
(JSC::Wasm::B3IRGenerator::addCurrentMemory):
(JSC::Wasm::B3IRGenerator::addCallIndirect):

• wasm/WasmBinding.cpp:

(JSC::Wasm::wasmToWasm):

• wasm/WasmInstance.h:

(JSC::Wasm::Instance::cachedMemory const):
(JSC::Wasm::Instance::cachedMemorySize const):
(JSC::Wasm::Instance::createWeakPtr):
(JSC::Wasm::Instance::setMemory):
(JSC::Wasm::Instance::updateCachedMemory):
(JSC::Wasm::Instance::offsetOfCachedMemory):
(JSC::Wasm::Instance::offsetOfCachedMemorySize):
(JSC::Wasm::Instance::offsetOfCachedIndexingMask):
(JSC::Wasm::Instance::allocationSize):

• wasm/WasmMemory.cpp:

(JSC::Wasm::Memory::grow):
(JSC::Wasm::Memory::registerInstance):

• wasm/WasmMemory.h:

(JSC::Wasm::Memory::indexingMask):

• wasm/js/JSToWasm.cpp:

(JSC::Wasm::createJSToWasmWrapper):

• wasm/js/WebAssemblyModuleRecord.cpp:

(JSC::WebAssemblyModuleRecord::evaluate):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/wasm/function-tests/memory-reuse.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp (modified) (3 diffs)
• Source/JavaScriptCore/wasm/WasmBinding.cpp (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmInstance.h (modified) (4 diffs)
• Source/JavaScriptCore/wasm/WasmMemory.cpp (modified) (3 diffs)
• Source/JavaScriptCore/wasm/WasmMemory.h (modified) (6 diffs)
• Source/JavaScriptCore/wasm/js/JSToWasm.cpp (modified) (1 diff)
• Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2018-02-23  Oleksandr Skachkov  <gskachkov@gmail.com>
+
+        WebAssembly: cache memory address / size on instance
+        https://bugs.webkit.org/show_bug.cgi?id=177305
+
+        Reviewed by JF Bastien.
+
+        * wasm/function-tests/memory-reuse.js: Added.
+        (createWasmInstance):
+        (doCheckTrap):
+        (doMemoryGrow):
+        (doCheck):
+        (checkWasmInstancesWithSharedMemory):
+
 2018-02-23  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-02-23  Oleksandr Skachkov  <gskachkov@gmail.com>
+
+        WebAssembly: cache memory address / size on instance
+        https://bugs.webkit.org/show_bug.cgi?id=177305
+
+        Reviewed by JF Bastien.
+
+        Cache memory address/size in wasm:Instance to avoid load wasm:Memory 
+        object during access to memory and memory size property in JiT
+
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::restoreWebAssemblyGlobalState):
+        (JSC::Wasm::B3IRGenerator::addCurrentMemory):
+        (JSC::Wasm::B3IRGenerator::addCallIndirect):
+        * wasm/WasmBinding.cpp:
+        (JSC::Wasm::wasmToWasm):
+        * wasm/WasmInstance.h:
+        (JSC::Wasm::Instance::cachedMemory const):
+        (JSC::Wasm::Instance::cachedMemorySize const):
+        (JSC::Wasm::Instance::createWeakPtr):
+        (JSC::Wasm::Instance::setMemory):
+        (JSC::Wasm::Instance::updateCachedMemory):
+        (JSC::Wasm::Instance::offsetOfCachedMemory):
+        (JSC::Wasm::Instance::offsetOfCachedMemorySize):
+        (JSC::Wasm::Instance::offsetOfCachedIndexingMask):
+        (JSC::Wasm::Instance::allocationSize):
+        * wasm/WasmMemory.cpp:
+        (JSC::Wasm::Memory::grow):
+        (JSC::Wasm::Memory::registerInstance):
+        * wasm/WasmMemory.h:
+        (JSC::Wasm::Memory::indexingMask):
+        * wasm/js/JSToWasm.cpp:
+        (JSC::Wasm::createJSToWasmWrapper):
+        * wasm/js/WebAssemblyModuleRecord.cpp:
+        (JSC::WebAssemblyModuleRecord::evaluate):
+
 2018-02-23  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

@@ -486 +486 @@
         patchpoint->setGenerator([pinnedRegs] (CCallHelpers& jit, const B3::StackmapGenerationParams& params) {
             GPRReg baseMemory = pinnedRegs->baseMemoryPointer;
-            jit.loadPtr(CCallHelpers::Address(params[0].gpr(), Instance::offsetOfMemory()), baseMemory);
             const auto& sizeRegs = pinnedRegs->sizeRegisters;
             ASSERT(sizeRegs.size() >= 1);
             ASSERT(!sizeRegs[0].sizeOffset); // The following code assumes we start at 0, and calculates subsequent size registers relative to 0.
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfSize()), sizeRegs[0].sizeRegister);
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfIndexingMask()), pinnedRegs->indexingMask);
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfMemory()), baseMemory);
+            jit.loadPtr(CCallHelpers::Address(params[0].gpr(), Instance::offsetOfCachedMemorySize()), sizeRegs[0].sizeRegister);
+            jit.loadPtr(CCallHelpers::Address(params[0].gpr(), Instance::offsetOfCachedIndexingMask()), pinnedRegs->indexingMask);
+            jit.loadPtr(CCallHelpers::Address(params[0].gpr(), Instance::offsetOfCachedMemory()), baseMemory);
             for (unsigned i = 1; i < sizeRegs.size(); ++i)
                 jit.add64(CCallHelpers::TrustedImm32(-sizeRegs[i].sizeOffset), sizeRegs[0].sizeRegister, sizeRegs[i].sizeRegister);
@@ -605 +604 @@
 auto B3IRGenerator::addCurrentMemory(ExpressionType& result) -> PartialResult
 {
-    Value* memoryObject = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), instanceValue(), safeCast<int32_t>(Instance::offsetOfMemory()));
-
     static_assert(sizeof(decltype(static_cast<Memory*>(nullptr)->size())) == sizeof(uint64_t), "codegen relies on this size");
-    Value* size = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, Int64, origin(), memoryObject, safeCast<int32_t>(Memory::offsetOfSize()));
-    
+    Value* size = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, Int64, origin(), instanceValue(), safeCast<int32_t>(Instance::offsetOfCachedMemorySize()));
+
     constexpr uint32_t shiftValue = 16;
     static_assert(PageCount::pageSize == 1ull << shiftValue, "This must hold for the code below to be correct.");
@@ -1300 +1297 @@
             jit.storePtr(baseMemory, CCallHelpers::Address(newContextInstance, Instance::offsetOfCachedStackLimit()));
             jit.storeWasmContextInstance(newContextInstance);
-            jit.loadPtr(CCallHelpers::Address(newContextInstance, Instance::offsetOfMemory()), baseMemory); // Memory*.
+            ASSERT(sizeRegs[0].sizeRegister != baseMemory);
+            // FIXME: We should support more than one memory size register
+            //   see: https://bugs.webkit.org/show_bug.cgi?id=162952
             ASSERT(sizeRegs.size() == 1);
-            ASSERT(sizeRegs[0].sizeRegister != baseMemory);
             ASSERT(sizeRegs[0].sizeRegister != newContextInstance);
             ASSERT(!sizeRegs[0].sizeOffset);
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfIndexingMask()), pinnedRegs.indexingMask); // Indexing mask.
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfSize()), sizeRegs[0].sizeRegister); // Memory size.
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfMemory()), baseMemory); // Memory::void*.
+            jit.loadPtr(CCallHelpers::Address(newContextInstance, Instance::offsetOfCachedIndexingMask()), pinnedRegs.indexingMask); // Indexing mask.
+            jit.loadPtr(CCallHelpers::Address(newContextInstance, Instance::offsetOfCachedMemorySize()), sizeRegs[0].sizeRegister); // Memory size.
+            jit.loadPtr(CCallHelpers::Address(newContextInstance, Instance::offsetOfCachedMemory()), baseMemory); // Memory::void*.
         });
         doContextSwitch->appendNewControlValue(m_proc, Jump, origin(), continuation);

trunk/Source/JavaScriptCore/wasm/WasmBinding.cpp

@@ -65 +65 @@
     // FIXME the following code assumes that all Wasm::Instance have the same pinned registers. https://bugs.webkit.org/show_bug.cgi?id=162952
     // Set up the callee's baseMemory register as well as the memory size registers.
-    jit.loadPtr(JIT::Address(baseMemory, Instance::offsetOfMemory()), baseMemory); // Wasm::Memory*.
     ASSERT(!sizeRegs[0].sizeOffset); // The following code assumes we start at 0, and calculates subsequent size registers relative to 0.
-    jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfIndexingMask()), pinnedRegs.indexingMask); // Indexing mask.
-    jit.loadPtr(JIT::Address(baseMemory, Wasm::Memory::offsetOfSize()), sizeRegs[0].sizeRegister); // Memory size.
-    jit.loadPtr(JIT::Address(baseMemory, Wasm::Memory::offsetOfMemory()), baseMemory); // Wasm::Memory::void*.
+    jit.loadPtr(JIT::Address(baseMemory, Wasm::Instance::offsetOfCachedIndexingMask()), pinnedRegs.indexingMask); // Indexing mask.
+    jit.loadPtr(JIT::Address(baseMemory, Wasm::Instance::offsetOfCachedMemorySize()), sizeRegs[0].sizeRegister); // Memory size.
+    jit.loadPtr(JIT::Address(baseMemory, Wasm::Instance::offsetOfCachedMemory()), baseMemory); // Wasm::Memory::void*.
     for (unsigned i = 1; i < sizeRegs.size(); ++i) {
         ASSERT(sizeRegs[i].sizeRegister != baseMemory);

trunk/Source/JavaScriptCore/wasm/WasmInstance.h

@@ -66 +66 @@
     Memory* memory() { return m_memory.get(); }
     Table* table() { return m_table.get(); }
-    
-    void setMemory(Ref<Memory>&& memory) { m_memory = WTFMove(memory); }
+
+    void* cachedMemory() const { return m_cachedMemory; }
+    size_t cachedMemorySize() const { return m_cachedMemorySize; }
+
+    WeakPtr<Instance> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(*this); }
+    void setMemory(Ref<Memory>&& memory)
+    {
+        m_memory = WTFMove(memory);
+        m_memory.get()->registerInstance(this);
+        updateCachedMemory();
+    }
+    void updateCachedMemory()
+    {
+        if (m_memory != nullptr) {
+            m_cachedMemory = memory()->memory();
+            m_cachedMemorySize = memory()->size();
+            m_cachedIndexingMask = memory()->indexingMask();
+        }
+    }
     void setTable(Ref<Table>&& table) { m_table = WTFMove(table); }
 
@@ -79 +96 @@
     static ptrdiff_t offsetOfGlobals() { return OBJECT_OFFSETOF(Instance, m_globals); }
     static ptrdiff_t offsetOfTable() { return OBJECT_OFFSETOF(Instance, m_table); }
+    static ptrdiff_t offsetOfCachedMemory() { return OBJECT_OFFSETOF(Instance, m_cachedMemory); }
+    static ptrdiff_t offsetOfCachedMemorySize() { return OBJECT_OFFSETOF(Instance, m_cachedMemorySize); }
+    static ptrdiff_t offsetOfCachedIndexingMask() { return OBJECT_OFFSETOF(Instance, m_cachedIndexingMask); }
     static ptrdiff_t offsetOfPointerToTopEntryFrame() { return OBJECT_OFFSETOF(Instance, m_pointerToTopEntryFrame); }
 
@@ -127 +147 @@
         return (offsetOfTail() + sizeof(ImportFunctionInfo) * numImportFunctions).unsafeGet();
     }
-
     void* m_owner { nullptr }; // In a JS embedding, this is a JSWebAssemblyInstance*.
     Context* m_context { nullptr };
+    void* m_cachedMemory { nullptr };
+    size_t m_cachedMemorySize { 0 };
+    size_t m_cachedIndexingMask { 0 };
     Ref<Module> m_module;
     RefPtr<CodeBlock> m_codeBlock;
@@ -139 +161 @@
     void* m_cachedStackLimit { bitwise_cast<void*>(std::numeric_limits<uintptr_t>::max()) };
     StoreTopCallFrameCallback m_storeTopCallFrame;
+    WeakPtrFactory<Instance> m_weakPtrFactory;
     unsigned m_numImportFunctions { 0 };
 };

trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp

@@ -26 +26 @@
 #include "config.h"
 #include "WasmMemory.h"
+#include "WasmInstance.h"
 
 #if ENABLE(WEBASSEMBLY)
@@ -378 +379 @@
     auto success = [&] () {
         m_growSuccessCallback(GrowSuccessTag, oldPageCount, newPageCount);
+        // Update cache for instance
+        for (auto& instance : m_instances) {
+            if (instance.get() != nullptr)
+                instance.get()->updateCachedMemory();
+        }
         return oldPageCount;
     };
@@ -438 +444 @@
 }
 
+void Memory::registerInstance(Instance* instance)
+{
+    size_t count = m_instances.size();
+    for (size_t index = 0; index < count; index++) {
+        if (m_instances.at(index).get() == nullptr) {
+            m_instances.at(index) = instance->createWeakPtr();
+            return;
+        }
+    }
+    m_instances.append(instance->createWeakPtr());
+}
+
 void Memory::dump(PrintStream& out) const
 {

trunk/Source/JavaScriptCore/wasm/WasmMemory.h

@@ -35 +35 @@
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
+#include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
 
 namespace WTF {
@@ -43 +45 @@
 
 namespace Wasm {
+
+class Instance;
 
 class Memory : public RefCounted<Memory> {
@@ -67 +71 @@
     void* memory() const { return m_memory; }
     size_t size() const { return m_size; }
+    size_t indexingMask() { return m_indexingMask; }
     PageCount sizeInPages() const { return PageCount::fromBytes(m_size); }
 
@@ -81 +86 @@
     };
     Expected<PageCount, GrowFailReason> grow(PageCount);
+    void registerInstance(Instance*);
 
     void check() {  ASSERT(!deletionHasBegun()); }
@@ -93 +99 @@
     Memory(PageCount initial, PageCount maximum, WTF::Function<void(NotifyPressure)>&& notifyMemoryPressure, WTF::Function<void(SyncTryToReclaim)>&& syncTryToReclaimMemory, WTF::Function<void(GrowSuccess, PageCount, PageCount)>&& growSuccessCallback);
 
-    // FIXME: we cache these on the instances to avoid a load on instance->instance calls. This will require updating all the instances when grow is called. https://bugs.webkit.org/show_bug.cgi?id=177305
     void* m_memory { nullptr };
     size_t m_size { 0 };
@@ -104 +109 @@
     WTF::Function<void(SyncTryToReclaim)> m_syncTryToReclaimMemory;
     WTF::Function<void(GrowSuccess, PageCount, PageCount)> m_growSuccessCallback;
+    Vector<WeakPtr<Instance>> m_instances;
 };
 

trunk/Source/JavaScriptCore/wasm/js/JSToWasm.cpp

@@ -176 +176 @@
         GPRReg baseMemory = pinnedRegs.baseMemoryPointer;
 
-        if (!Context::useFastTLS())
-            jit.loadPtr(CCallHelpers::Address(wasmContextInstanceGPR, Instance::offsetOfMemory()), baseMemory);
-        else {
+        if (Context::useFastTLS())
             jit.loadWasmContextInstance(baseMemory);
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Instance::offsetOfMemory()), baseMemory);
-        }
-
+
+        GPRReg currentInstanceGPR = Context::useFastTLS() ? baseMemory : wasmContextInstanceGPR;
         if (mode != MemoryMode::Signaling) {
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Wasm::Memory::offsetOfIndexingMask()), pinnedRegs.indexingMask);
+            jit.loadPtr(CCallHelpers::Address(currentInstanceGPR, Wasm::Instance::offsetOfCachedIndexingMask()), pinnedRegs.indexingMask);
             const auto& sizeRegs = pinnedRegs.sizeRegisters;
             ASSERT(sizeRegs.size() >= 1);
             ASSERT(!sizeRegs[0].sizeOffset); // The following code assumes we start at 0, and calculates subsequent size registers relative to 0.
-            jit.loadPtr(CCallHelpers::Address(baseMemory, Wasm::Memory::offsetOfSize()), sizeRegs[0].sizeRegister);
+            jit.loadPtr(CCallHelpers::Address(currentInstanceGPR, Wasm::Instance::offsetOfCachedMemorySize()), sizeRegs[0].sizeRegister);
             for (unsigned i = 1; i < sizeRegs.size(); ++i)
                 jit.add64(CCallHelpers::TrustedImm32(-sizeRegs[i].sizeOffset), sizeRegs[0].sizeRegister, sizeRegs[i].sizeRegister);
         }
 
-        jit.loadPtr(CCallHelpers::Address(baseMemory, Wasm::Memory::offsetOfMemory()), baseMemory);
+        jit.loadPtr(CCallHelpers::Address(currentInstanceGPR, Wasm::Instance::offsetOfCachedMemory()), baseMemory);
     }
 

trunk/Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp

@@ -224 +224 @@
 
     const Vector<Wasm::Segment::Ptr>& data = moduleInformation.data;
-    JSWebAssemblyMemory* jsMemory = m_instance->memory();
-
+    
     std::optional<JSValue> exception;
 
@@ -251 +250 @@
 
     auto forEachSegment = [&] (auto fn) {
-        uint8_t* memory = reinterpret_cast<uint8_t*>(jsMemory->memory().memory());
-        uint64_t sizeInBytes = jsMemory->memory().size();
+        uint8_t* memory = reinterpret_cast<uint8_t*>(m_instance->instance().cachedMemory());
+        uint64_t sizeInBytes = m_instance->instance().cachedMemorySize();
 
         for (const Wasm::Segment::Ptr& segment : data) {