Commit 0a61361e authored by titzer's avatar titzer Committed by Commit Bot

[wasm] Use WeakFixedArray for list of instances sharing a WasmMemoryObject.

This CL refactors the WasmMemoryObject and WasmInstanceObject classes to
use WeakFixedArray instead of using a doubly-linked list of instances. This
simplifies the lifetime management of instances by not requiring them to
be unlinked from this list upon GC. It also simplifies the iteration over
the instances using a given WasmMemoryObject.

Note that, contrary to my naive assumption at the outset, it is still necessary for the InstanceFinalizer (called upon a WasmInstanceObject death) to unlink itself from a WasmMemoryObject's instances list, due to finalizer ordering.

R=deepti@chromium.org, mlippautz@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2972803002
Cr-Commit-Position: refs/heads/master@{#46482}
parent 0a4b65ff
......@@ -1022,9 +1022,9 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
//--------------------------------------------------------------------------
// Add instance to Memory object
//--------------------------------------------------------------------------
DCHECK(instance->IsWasmInstanceObject());
if (instance->has_memory_object()) {
instance->memory_object()->AddInstance(isolate_, instance);
Handle<WasmMemoryObject> memory(instance->memory_object(), isolate_);
WasmMemoryObject::AddInstance(isolate_, memory, instance);
}
//--------------------------------------------------------------------------
......@@ -1667,7 +1667,6 @@ void InstanceBuilder::ProcessExports(
} else {
memory_object =
Handle<WasmMemoryObject>(instance->memory_object(), isolate_);
memory_object->ResetInstancesLink(isolate_);
}
desc.set_value(memory_object);
......
......@@ -88,48 +88,6 @@ void* TryAllocateBackingStore(Isolate* isolate, size_t size,
}
}
static void MemoryInstanceFinalizer(Isolate* isolate,
WasmInstanceObject* instance) {
DisallowHeapAllocation no_gc;
// If the memory object is destroyed, nothing needs to be done here.
if (!instance->has_memory_object()) return;
Handle<WasmInstanceWrapper> instance_wrapper =
handle(instance->instance_wrapper());
DCHECK(WasmInstanceWrapper::IsWasmInstanceWrapper(*instance_wrapper));
DCHECK(instance_wrapper->has_instance());
bool has_prev = instance_wrapper->has_previous();
bool has_next = instance_wrapper->has_next();
Handle<WasmMemoryObject> memory_object(instance->memory_object());
if (!has_prev && !has_next) {
memory_object->ResetInstancesLink(isolate);
return;
} else {
Handle<WasmInstanceWrapper> next_wrapper, prev_wrapper;
if (!has_prev) {
Handle<WasmInstanceWrapper> next_wrapper =
instance_wrapper->next_wrapper();
next_wrapper->reset_previous_wrapper();
// As this is the first link in the memory object, destroying
// without updating memory object would corrupt the instance chain in
// the memory object.
memory_object->set_instances_link(*next_wrapper);
} else if (!has_next) {
instance_wrapper->previous_wrapper()->reset_next_wrapper();
} else {
DCHECK(has_next && has_prev);
Handle<WasmInstanceWrapper> prev_wrapper =
instance_wrapper->previous_wrapper();
Handle<WasmInstanceWrapper> next_wrapper =
instance_wrapper->next_wrapper();
prev_wrapper->set_next_wrapper(*next_wrapper);
next_wrapper->set_previous_wrapper(*prev_wrapper);
}
// Reset to avoid dangling pointers
instance_wrapper->reset();
}
}
static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
DisallowHeapAllocation no_gc;
JSObject** p = reinterpret_cast<JSObject**>(data.GetParameter());
......@@ -137,7 +95,6 @@ static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
// If a link to shared memory instances exists, update the list of memory
// instances before the instance is destroyed.
if (owner->has_instance_wrapper()) MemoryInstanceFinalizer(isolate, owner);
WasmCompiledModule* compiled_module = owner->compiled_module();
TRACE("Finalizing %d {\n", compiled_module->instance_id());
DCHECK(compiled_module->has_weak_wasm_module());
......@@ -155,6 +112,18 @@ static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
}
}
// Since the order of finalizers is not guaranteed, it can be the case
// that {instance->compiled_module()->module()}, which is a
// {Managed<WasmModule>} has been collected earlier in this GC cycle.
// Weak references to this instance won't be cleared until
// the next GC cycle, so we need to manually break some links (such as
// the weak references from {WasmMemoryObject::instances}.
if (owner->has_memory_object()) {
Handle<WasmMemoryObject> memory(owner->memory_object(), isolate);
Handle<WasmInstanceObject> instance(owner, isolate);
WasmMemoryObject::RemoveInstance(isolate, memory, instance);
}
// weak_wasm_module may have been cleared, meaning the module object
// was GC-ed. In that case, there won't be any new instances created,
// and we don't need to maintain the links between instances.
......
......@@ -323,7 +323,7 @@ Handle<WasmMemoryObject> WasmMemoryObject::New(Isolate* isolate,
}
memory_obj->set_array_buffer(*buffer);
memory_obj->set_maximum_pages(maximum);
return Handle<WasmMemoryObject>::cast(memory_obj);
return memory_obj;
}
uint32_t WasmMemoryObject::current_pages() {
......@@ -333,17 +333,23 @@ uint32_t WasmMemoryObject::current_pages() {
}
void WasmMemoryObject::AddInstance(Isolate* isolate,
Handle<WasmMemoryObject> memory,
Handle<WasmInstanceObject> instance) {
Handle<WasmInstanceWrapper> instance_wrapper =
handle(instance->instance_wrapper());
if (has_instances_link()) {
Handle<WasmInstanceWrapper> current_wrapper(instances_link());
DCHECK(WasmInstanceWrapper::IsWasmInstanceWrapper(*current_wrapper));
DCHECK(!current_wrapper->has_previous());
instance_wrapper->set_next_wrapper(*current_wrapper);
current_wrapper->set_previous_wrapper(*instance_wrapper);
Handle<WeakFixedArray> old_instances =
memory->has_instances()
? Handle<WeakFixedArray>(memory->instances(), isolate)
: Handle<WeakFixedArray>::null();
Handle<WeakFixedArray> new_instances =
WeakFixedArray::Add(old_instances, instance);
memory->set_instances(*new_instances);
}
void WasmMemoryObject::RemoveInstance(Isolate* isolate,
Handle<WasmMemoryObject> memory,
Handle<WasmInstanceObject> instance) {
if (memory->has_instances()) {
memory->instances()->Remove(instance);
}
set_instances_link(*instance_wrapper);
}
// static
......@@ -368,42 +374,30 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
DCHECK_EQ(0, old_size % WasmModule::kPageSize);
return old_size / WasmModule::kPageSize;
}
if (!memory_object->has_instances_link()) {
// Memory object does not have an instance associated with it, just grow
uint32_t max_pages;
if (memory_object->has_maximum_pages()) {
max_pages = static_cast<uint32_t>(memory_object->maximum_pages());
if (FLAG_wasm_max_mem_pages < max_pages) return -1;
} else {
max_pages = FLAG_wasm_max_mem_pages;
}
new_buffer = GrowMemoryBuffer(isolate, old_buffer, pages, max_pages);
if (new_buffer.is_null()) return -1;
uint32_t max_pages;
if (memory_object->has_maximum_pages()) {
max_pages = static_cast<uint32_t>(memory_object->maximum_pages());
if (FLAG_wasm_max_mem_pages < max_pages) return -1;
} else {
Handle<WasmInstanceWrapper> instance_wrapper(
memory_object->instances_link());
DCHECK(WasmInstanceWrapper::IsWasmInstanceWrapper(*instance_wrapper));
DCHECK(instance_wrapper->has_instance());
Handle<WasmInstanceObject> instance = instance_wrapper->instance_object();
DCHECK(instance->IsWasmInstanceObject());
uint32_t max_pages = instance->GetMaxMemoryPages();
// Grow memory object buffer and update instances associated with it.
new_buffer = GrowMemoryBuffer(isolate, old_buffer, pages, max_pages);
if (new_buffer.is_null()) return -1;
DCHECK(!instance_wrapper->has_previous());
SetInstanceMemory(isolate, instance, new_buffer);
max_pages = FLAG_wasm_max_mem_pages;
}
new_buffer = GrowMemoryBuffer(isolate, old_buffer, pages, max_pages);
if (new_buffer.is_null()) return -1;
if (memory_object->has_instances()) {
Address old_mem_start = static_cast<Address>(old_buffer->backing_store());
UncheckedUpdateInstanceMemory(isolate, instance, old_mem_start, old_size);
while (instance_wrapper->has_next()) {
instance_wrapper = instance_wrapper->next_wrapper();
DCHECK(WasmInstanceWrapper::IsWasmInstanceWrapper(*instance_wrapper));
Handle<WasmInstanceObject> instance = instance_wrapper->instance_object();
DCHECK(instance->IsWasmInstanceObject());
Handle<WeakFixedArray> instances(memory_object->instances(), isolate);
for (int i = 0; i < instances->Length(); i++) {
Object* elem = instances->Get(i);
if (!elem->IsWasmInstanceObject()) continue;
Handle<WasmInstanceObject> instance(WasmInstanceObject::cast(elem),
isolate);
SetInstanceMemory(isolate, instance, new_buffer);
UncheckedUpdateInstanceMemory(isolate, instance, old_mem_start, old_size);
}
}
memory_object->set_array_buffer(*new_buffer);
DCHECK_EQ(0, old_size % WasmModule::kPageSize);
return old_size / WasmModule::kPageSize;
......@@ -434,9 +428,6 @@ Handle<WasmInstanceObject> WasmInstanceObject::New(
reinterpret_cast<WasmInstanceObject*>(*instance_object), isolate);
instance->set_compiled_module(*compiled_module);
Handle<WasmInstanceWrapper> instance_wrapper =
WasmInstanceWrapper::New(isolate, instance);
instance->set_instance_wrapper(*instance_wrapper);
return instance;
}
......@@ -1392,29 +1383,3 @@ Handle<Code> WasmCompiledModule::CompileLazy(
return orch->CompileLazy(isolate, instance, caller, offset, func_index,
patch_caller);
}
Handle<WasmInstanceWrapper> WasmInstanceWrapper::New(
Isolate* isolate, Handle<WasmInstanceObject> instance) {
Handle<FixedArray> array =
isolate->factory()->NewFixedArray(kFieldCount, TENURED);
Handle<WasmInstanceWrapper> instance_wrapper(
reinterpret_cast<WasmInstanceWrapper*>(*array), isolate);
Handle<WeakCell> cell = isolate->factory()->NewWeakCell(instance);
instance_wrapper->set(kWrapperInstanceObjectIndex, *cell);
return instance_wrapper;
}
bool WasmInstanceWrapper::IsWasmInstanceWrapper(Object* obj) {
if (!obj->IsFixedArray()) return false;
Handle<FixedArray> array = handle(FixedArray::cast(obj));
if (array->length() != kFieldCount) return false;
if (!array->get(kWrapperInstanceObjectIndex)->IsWeakCell()) return false;
Isolate* isolate = array->GetIsolate();
if (!array->get(kNextInstanceWrapperIndex)->IsUndefined(isolate) &&
!array->get(kNextInstanceWrapperIndex)->IsFixedArray())
return false;
if (!array->get(kPreviousInstanceWrapperIndex)->IsUndefined(isolate) &&
!array->get(kPreviousInstanceWrapperIndex)->IsFixedArray())
return false;
return true;
}
......@@ -29,7 +29,6 @@ class WasmInterpreter;
class WasmCompiledModule;
class WasmDebugInfo;
class WasmInstanceObject;
class WasmInstanceWrapper;
#define DECL_OOL_QUERY(type) static bool Is##type(Object* object);
#define DECL_OOL_CAST(type) static type* cast(Object* object);
......@@ -113,22 +112,26 @@ class WasmMemoryObject : public JSObject {
DECL_ACCESSORS(array_buffer, JSArrayBuffer)
DECL_INT_ACCESSORS(maximum_pages)
DECL_OPTIONAL_ACCESSORS(instances_link, WasmInstanceWrapper)
DECL_OPTIONAL_ACCESSORS(instances, WeakFixedArray)
enum { // --
kArrayBufferIndex,
kMaximumPagesIndex,
kInstancesLinkIndex,
kInstancesIndex,
kFieldCount
};
DEF_SIZE(JSObject)
DEF_OFFSET(ArrayBuffer)
DEF_OFFSET(MaximumPages)
DEF_OFFSET(InstancesLink)
void AddInstance(Isolate* isolate, Handle<WasmInstanceObject> object);
inline void ResetInstancesLink(Isolate* isolate);
DEF_OFFSET(Instances)
// Add an instance to the internal (weak) list. amortized O(n).
static void AddInstance(Isolate* isolate, Handle<WasmMemoryObject> memory,
Handle<WasmInstanceObject> object);
// Remove an instance from the internal (weak) list. O(n).
static void RemoveInstance(Isolate* isolate, Handle<WasmMemoryObject> memory,
Handle<WasmInstanceObject> object);
uint32_t current_pages();
inline bool has_maximum_pages() { return maximum_pages() >= 0; }
......@@ -149,7 +152,6 @@ class WasmInstanceObject : public JSObject {
DECL_OPTIONAL_ACCESSORS(memory_buffer, JSArrayBuffer)
DECL_OPTIONAL_ACCESSORS(globals_buffer, JSArrayBuffer)
DECL_OPTIONAL_ACCESSORS(debug_info, WasmDebugInfo)
DECL_OPTIONAL_ACCESSORS(instance_wrapper, WasmInstanceWrapper)
// FixedArray of all instances whose code was imported
DECL_OPTIONAL_ACCESSORS(directly_called_instances, FixedArray)
......@@ -159,7 +161,6 @@ class WasmInstanceObject : public JSObject {
kMemoryBufferIndex,
kGlobalsBufferIndex,
kDebugInfoIndex,
kInstanceWrapperIndex,
kDirectlyCalledInstancesIndex,
kFieldCount
};
......@@ -170,7 +171,6 @@ class WasmInstanceObject : public JSObject {
DEF_OFFSET(MemoryBuffer)
DEF_OFFSET(GlobalsBuffer)
DEF_OFFSET(DebugInfo)
DEF_OFFSET(InstanceWrapper)
DEF_OFFSET(DirectlyCalledInstances)
WasmModuleObject* module_object();
......@@ -668,64 +668,6 @@ class WasmDebugInfo : public FixedArray {
int frame_index);
};
class WasmInstanceWrapper : public FixedArray {
public:
enum { // --
kWrapperInstanceObjectIndex,
kNextInstanceWrapperIndex,
kPreviousInstanceWrapperIndex,
kFieldCount
};
static WasmInstanceWrapper* cast(Object* fixed_array) {
SLOW_DCHECK(IsWasmInstanceWrapper(fixed_array));
return reinterpret_cast<WasmInstanceWrapper*>(fixed_array);
}
static bool IsWasmInstanceWrapper(Object* obj);
bool has_instance() { return get(kWrapperInstanceObjectIndex)->IsWeakCell(); }
Handle<WasmInstanceObject> instance_object() {
Object* obj = get(kWrapperInstanceObjectIndex);
DCHECK(obj->IsWeakCell());
WeakCell* cell = WeakCell::cast(obj);
DCHECK(cell->value()->IsJSObject());
return handle(WasmInstanceObject::cast(cell->value()));
}
bool has_next() {
return IsWasmInstanceWrapper(get(kNextInstanceWrapperIndex));
}
bool has_previous() {
return IsWasmInstanceWrapper(get(kPreviousInstanceWrapperIndex));
}
void set_next_wrapper(Object* obj) {
DCHECK(IsWasmInstanceWrapper(obj));
set(kNextInstanceWrapperIndex, obj);
}
void set_previous_wrapper(Object* obj) {
DCHECK(IsWasmInstanceWrapper(obj));
set(kPreviousInstanceWrapperIndex, obj);
}
Handle<WasmInstanceWrapper> next_wrapper() {
Object* obj = get(kNextInstanceWrapperIndex);
DCHECK(IsWasmInstanceWrapper(obj));
return handle(WasmInstanceWrapper::cast(obj));
}
Handle<WasmInstanceWrapper> previous_wrapper() {
Object* obj = get(kPreviousInstanceWrapperIndex);
DCHECK(IsWasmInstanceWrapper(obj));
return handle(WasmInstanceWrapper::cast(obj));
}
void reset_next_wrapper() { set_undefined(kNextInstanceWrapperIndex); }
void reset_previous_wrapper() {
set_undefined(kPreviousInstanceWrapperIndex);
}
void reset() {
for (int kID = 0; kID < kFieldCount; kID++) set_undefined(kID);
}
static Handle<WasmInstanceWrapper> New(Isolate* isolate,
Handle<WasmInstanceObject> instance);
};
// TODO(titzer): these should be moved to wasm-objects-inl.h
CAST_ACCESSOR(WasmInstanceObject)
CAST_ACCESSOR(WasmMemoryObject)
......@@ -744,8 +686,7 @@ ACCESSORS(WasmTableObject, dispatch_tables, FixedArray, kDispatchTablesOffset)
// WasmMemoryObject
ACCESSORS(WasmMemoryObject, array_buffer, JSArrayBuffer, kArrayBufferOffset)
SMI_ACCESSORS(WasmMemoryObject, maximum_pages, kMaximumPagesOffset)
ACCESSORS(WasmMemoryObject, instances_link, WasmInstanceWrapper,
kInstancesLinkOffset)
ACCESSORS(WasmMemoryObject, instances, WeakFixedArray, kInstancesOffset)
// WasmInstanceObject
ACCESSORS(WasmInstanceObject, compiled_module, WasmCompiledModule,
......@@ -756,8 +697,6 @@ ACCESSORS(WasmInstanceObject, memory_buffer, JSArrayBuffer, kMemoryBufferOffset)
ACCESSORS(WasmInstanceObject, globals_buffer, JSArrayBuffer,
kGlobalsBufferOffset)
ACCESSORS(WasmInstanceObject, debug_info, WasmDebugInfo, kDebugInfoOffset)
ACCESSORS(WasmInstanceObject, instance_wrapper, WasmInstanceWrapper,
kInstanceWrapperOffset)
ACCESSORS(WasmInstanceObject, directly_called_instances, FixedArray,
kDirectlyCalledInstancesOffset)
......@@ -782,9 +721,8 @@ ACCESSORS(WasmSharedModuleData, breakpoint_infos, FixedArray,
OPTIONAL_ACCESSOR(WasmInstanceObject, debug_info, kDebugInfoOffset)
OPTIONAL_ACCESSOR(WasmInstanceObject, memory_buffer, kMemoryBufferOffset)
OPTIONAL_ACCESSOR(WasmInstanceObject, memory_object, kMemoryObjectOffset)
OPTIONAL_ACCESSOR(WasmInstanceObject, instance_wrapper, kInstanceWrapperOffset)
OPTIONAL_ACCESSOR(WasmMemoryObject, instances_link, kInstancesLinkOffset)
OPTIONAL_ACCESSOR(WasmMemoryObject, instances, kInstancesOffset)
OPTIONAL_ACCESSOR(WasmSharedModuleData, breakpoint_infos,
kBreakPointInfosOffset)
......@@ -797,11 +735,6 @@ ACCESSORS(WasmDebugInfo, locals_names, FixedArray, kLocalsNamesOffset)
OPTIONAL_ACCESSOR(WasmDebugInfo, locals_names, kLocalsNamesOffset)
inline void WasmMemoryObject::ResetInstancesLink(Isolate* isolate) {
// This has to be a raw access to bypass typechecking.
WRITE_FIELD(this, kInstancesLinkOffset, isolate->heap()->undefined_value());
}
#undef DECL_OOL_QUERY
#undef DECL_OOL_CAST
#undef DECL_GETTER
......
......@@ -24,17 +24,15 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
for (var i = 0; i < 5; i++) {
instances.push(builder.instantiate({mod: {imported_mem: memory}}));
}
function grow_instance_0(pages) { return instances[0].exports.grow(pages); }
function grow_instance_1(pages) { return instances[1].exports.grow(pages); }
function grow_instance_2(pages) { return instances[2].exports.grow(pages); }
function grow_instance_3(pages) { return instances[3].exports.grow(pages); }
function grow_instance_4(pages) { return instances[4].exports.grow(pages); }
function grow_instance(index, pages) {
return instances[index].exports.grow(pages);
}
var start_index = 0;
var end_index = 5;
function verify_mem_size(expected_pages) {
print(" checking size = " + expected_pages + " pages");
assertEquals(expected_pages*kPageSize, memory.buffer.byteLength);
for (var i = start_index; i < end_index; i++) {
for (let i = 0; i < instances.length; i++) {
if (instances[i] == null) continue;
assertEquals(expected_pages, instances[i].exports.mem_size());
}
}
......@@ -45,59 +43,49 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
assertEquals(5, memory.grow(6));
verify_mem_size(11);
print(" instances[1] = null, GC");
instances[1] = null;
gc();
// i[0] - i[2] - i[3] - i[4]
start_index = 2;
verify_mem_size(11);
assertEquals(11, instances[0].exports.mem_size());
assertEquals(11, grow_instance_2(10));
assertEquals(21*kPageSize, memory.buffer.byteLength);
assertEquals(11, grow_instance(2, 10));
verify_mem_size(21);
assertEquals(21, instances[0].exports.mem_size());
print(" instances[4] = null, GC");
instances[4] = null;
gc();
verify_mem_size(21);
// i[0] - i[2] - i[3]
assertEquals(21, instances[0].exports.mem_size());
assertEquals(21, instances[2].exports.mem_size());
assertEquals(21, instances[3].exports.mem_size());
assertEquals(21, memory.grow(2));
assertEquals(23*kPageSize, memory.buffer.byteLength);
assertEquals(23, instances[0].exports.mem_size());
assertEquals(23, instances[2].exports.mem_size());
assertEquals(23, instances[3].exports.mem_size());
verify_mem_size(23);
print(" instances[0] = null, GC");
instances[0] = null;
gc();
gc();
verify_mem_size(23);
// i[2] - i[3]
assertEquals(23, instances[2].exports.mem_size());
assertEquals(23, instances[3].exports.mem_size());
assertEquals(23, grow_instance_3(5));
assertEquals(28*kPageSize, memory.buffer.byteLength);
assertEquals(28, instances[2].exports.mem_size());
assertEquals(28, instances[3].exports.mem_size());
assertEquals(23, grow_instance(3, 5));
verify_mem_size(28);
print(" new instance, GC");
// Instantiate a new instance and verify that it can be grown correctly.
instances.push(builder.instantiate({mod: {imported_mem: memory}}));
function grow_instance_5(pages) { return instances[5].exports.grow(pages); }
gc();
gc();
// i[2] - i[3] - i[5]
assertEquals(28, instances[2].exports.mem_size());
assertEquals(28, instances[3].exports.mem_size());
assertEquals(28, instances[5].exports.mem_size());
assertEquals(28, grow_instance_5(2));
assertEquals(30*kPageSize, memory.buffer.byteLength);
assertEquals(30, instances[2].exports.mem_size());
assertEquals(30, instances[3].exports.mem_size());
assertEquals(30, instances[5].exports.mem_size());
verify_mem_size(28);
assertEquals(28, grow_instance(5, 2));
verify_mem_size(30);
assertEquals(30, memory.grow(5));
assertEquals(35*kPageSize, memory.buffer.byteLength);
assertEquals(35, instances[2].exports.mem_size());
assertEquals(35, instances[3].exports.mem_size());
assertEquals(35, instances[5].exports.mem_size());
verify_mem_size(35);
for (let i = 0; i < instances.length; i++) {
print(" instances[" + i + "] = null, GC");
instances[i] = null;
gc();
verify_mem_size(35);
}
})();
......@@ -623,4 +623,8 @@ class WasmModuleBuilder {
let instance = new WebAssembly.Instance(module, ffi);
return instance;
}
toModule(debug = false) {
return new WebAssembly.Module(this.toBuffer());
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment