Skip to content

Commit

Permalink
Merge r228966 - WebAssembly: cache memory address / size on instance
Browse files Browse the repository at this point in the history
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):
  • Loading branch information
gskachkov authored and carlosgcampos committed Mar 5, 2018
1 parent 796a0ed commit 3d4e80c
Show file tree
Hide file tree
Showing 10 changed files with 236 additions and 33 deletions.
14 changes: 14 additions & 0 deletions JSTests/ChangeLog
@@ -1,3 +1,17 @@
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>

[JSC] Implement $vm.ftlTrue function for FTL testing
Expand Down
113 changes: 113 additions & 0 deletions JSTests/wasm/function-tests/memory-reuse.js
@@ -0,0 +1,113 @@
import Builder from '../Builder.js'
import * as assert from '../assert.js'

const count = 10;
const pages = 6;
const memoryDescription = {initial:1};
const pageSize = 64 * 1024;

function createWasmInstance(memory) {
const builder = new Builder()
.Type().End()
.Import()
.Memory("imp", "memory", memoryDescription)
.End()
.Function().End()
.Export()
.Function("current")
.Function("get")
.Function("grow")
.End()
.Code()
.Function("current", { params: [], ret: "i32" })
.CurrentMemory(0)
.Return()
.End()
.Function("get", { params: ["i32"], ret: "i32" })
.GetLocal(0)
.I32Load(2, 0)
.Return()
.End()
.Function("grow", { params: ["i32"], ret: "i32" })
.GetLocal(0)
.GrowMemory(0)
.Return()
.End()
.End();

const bin = builder.WebAssembly().get();
const module = new WebAssembly.Module(bin);
return new WebAssembly.Instance(module, {imp: {memory}});
}

function doFillMemory(mem, length) {
const buf = new Uint32Array(mem.buffer);

for (let i = 0; i < length; ++i) {
buf[i * 4] = i + 1;
}
}

function doCheckTrap(instances, mem, numPages, length) {
const buf = new Uint32Array(mem.buffer);

for (let instance of instances) {
const foo = instance.exports.get;
for (let i = 0; i < length; i++) {
assert.eq(foo(i * 4), buf[i]);
}
assert.throws(() => foo(numPages * pageSize + 1), WebAssembly.RuntimeError, "Out of bounds memory access");
}
}

function doCheckNoTrap(instances, mem, numPages, length) {
const buf = new Uint32Array(mem.buffer);

for (let instance of instances) {
const foo = instance.exports.get;
for (let i = 0; i < length; i++) {
assert.eq(foo(i * 4), buf[i]);
}
assert.eq(foo(numPages * pageSize + 1), 0);
}
}

function doMemoryGrow(instances) {
const instance = instances[0]; // Take first instance and grow shared memory
instance.exports.grow(1);
}

function doCheck(mem, instances, numPages) {
const length = mem.buffer.byteLength / 4;

doFillMemory(mem, length);
doCheckTrap(instances, mem, numPages, length);
doMemoryGrow(instances);
doCheckNoTrap(instances, mem, numPages, length);
}

function checkWasmInstancesWithSharedMemory() {
const mem = new WebAssembly.Memory(memoryDescription);

const instances = [];
for (let i = 0; i < count; i++) {
instances.push(createWasmInstance(mem));
}

for(let i = 1; i < pages; i++) {
doCheck(mem, instances, i);
}

let instance;
for (let i = 0; i < count; i++) {
instance = instances.shift();
}

fullGC();

const survivedInstances = [ instance ]; // Should survive only one instance after full GC

doCheck(mem, survivedInstances, pages); // Recheck only for survie instances
}

checkWasmInstancesWithSharedMemory();
36 changes: 36 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,39 @@
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>

ArgumentsEliminationPhase has a branch on GetByOffset that should be an assert
Expand Down
24 changes: 11 additions & 13 deletions Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Expand Up @@ -485,13 +485,12 @@ void B3IRGenerator::restoreWebAssemblyGlobalState(RestoreCachedStackLimit restor

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);
});
Expand Down Expand Up @@ -604,11 +603,9 @@ auto B3IRGenerator::addGrowMemory(ExpressionType delta, ExpressionType& result)

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.");
Value* numPages = m_currentBlock->appendNew<Value>(m_proc, ZShr, origin(),
Expand Down Expand Up @@ -1299,14 +1296,15 @@ auto B3IRGenerator::addCallIndirect(const Signature& signature, Vector<Expressio
jit.loadPtr(CCallHelpers::Address(oldContextInstance, Instance::offsetOfCachedStackLimit()), baseMemory);
jit.storePtr(baseMemory, CCallHelpers::Address(newContextInstance, Instance::offsetOfCachedStackLimit()));
jit.storeWasmContextInstance(newContextInstance);
jit.loadPtr(CCallHelpers::Address(newContextInstance, Instance::offsetOfMemory()), baseMemory); // Memory*.
ASSERT(sizeRegs.size() == 1);
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 != 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);

Expand Down
7 changes: 3 additions & 4 deletions Source/JavaScriptCore/wasm/WasmBinding.cpp
Expand Up @@ -64,11 +64,10 @@ Expected<MacroAssemblerCodeRef, BindingFailure> wasmToWasm(unsigned importIndex)

// 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);
ASSERT(sizeRegs[i].sizeRegister != scratch);
Expand Down
29 changes: 26 additions & 3 deletions Source/JavaScriptCore/wasm/WasmInstance.h
Expand Up @@ -65,8 +65,25 @@ class Instance : public ThreadSafeRefCounted<Instance> {
CodeBlock* codeBlock() { return m_codeBlock.get(); }
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); }

int32_t loadI32Global(unsigned i) const { return m_globals.get()[i]; }
Expand All @@ -78,6 +95,9 @@ class Instance : public ThreadSafeRefCounted<Instance> {
static ptrdiff_t offsetOfMemory() { return OBJECT_OFFSETOF(Instance, m_memory); }
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); }

static ptrdiff_t offsetOfPointerToActualStackLimit() { return OBJECT_OFFSETOF(Instance, m_pointerToActualStackLimit); }
Expand Down Expand Up @@ -126,9 +146,11 @@ class Instance : public ThreadSafeRefCounted<Instance> {
{
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;
RefPtr<Memory> m_memory;
Expand All @@ -138,6 +160,7 @@ class Instance : public ThreadSafeRefCounted<Instance> {
void** m_pointerToActualStackLimit { nullptr };
void* m_cachedStackLimit { bitwise_cast<void*>(std::numeric_limits<uintptr_t>::max()) };
StoreTopCallFrameCallback m_storeTopCallFrame;
WeakPtrFactory<Instance> m_weakPtrFactory;
unsigned m_numImportFunctions { 0 };
};

Expand Down
18 changes: 18 additions & 0 deletions Source/JavaScriptCore/wasm/WasmMemory.cpp
Expand Up @@ -25,6 +25,7 @@

#include "config.h"
#include "WasmMemory.h"
#include "WasmInstance.h"

#if ENABLE(WEBASSEMBLY)

Expand Down Expand Up @@ -377,6 +378,11 @@ Expected<PageCount, Memory::GrowFailReason> Memory::grow(PageCount delta)

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;
};

Expand Down Expand Up @@ -437,6 +443,18 @@ Expected<PageCount, Memory::GrowFailReason> Memory::grow(PageCount delta)
return oldPageCount;
}

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
{
out.print("Memory at ", RawPointer(m_memory), ", size ", m_size, "B capacity ", m_mappedCapacity, "B, initial ", m_initial, " maximum ", m_maximum, " mode ", makeString(m_mode));
Expand Down
8 changes: 7 additions & 1 deletion Source/JavaScriptCore/wasm/WasmMemory.h
Expand Up @@ -34,6 +34,8 @@
#include <wtf/Function.h>
#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
#include <wtf/Vector.h>
#include <wtf/WeakPtr.h>

namespace WTF {
class PrintStream;
Expand All @@ -43,6 +45,8 @@ namespace JSC {

namespace Wasm {

class Instance;

class Memory : public RefCounted<Memory> {
WTF_MAKE_NONCOPYABLE(Memory);
WTF_MAKE_FAST_ALLOCATED;
Expand All @@ -66,6 +70,7 @@ class Memory : public RefCounted<Memory> {

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); }

PageCount initial() const { return m_initial; }
Expand All @@ -80,6 +85,7 @@ class Memory : public RefCounted<Memory> {
OutOfMemory,
};
Expected<PageCount, GrowFailReason> grow(PageCount);
void registerInstance(Instance*);

void check() { ASSERT(!deletionHasBegun()); }

Expand All @@ -92,7 +98,6 @@ class Memory : public RefCounted<Memory> {
Memory(void* memory, PageCount initial, PageCount maximum, size_t mappedCapacity, MemoryMode, WTF::Function<void(NotifyPressure)>&& notifyMemoryPressure, WTF::Function<void(SyncTryToReclaim)>&& syncTryToReclaimMemory, WTF::Function<void(GrowSuccess, PageCount, PageCount)>&& growSuccessCallback);
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 };
size_t m_indexingMask { 0 };
Expand All @@ -103,6 +108,7 @@ class Memory : public RefCounted<Memory> {
WTF::Function<void(NotifyPressure)> m_notifyMemoryPressure;
WTF::Function<void(SyncTryToReclaim)> m_syncTryToReclaimMemory;
WTF::Function<void(GrowSuccess, PageCount, PageCount)> m_growSuccessCallback;
Vector<WeakPtr<Instance>> m_instances;
};

} } // namespace JSC::Wasm
Expand Down

0 comments on commit 3d4e80c

Please sign in to comment.