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

src/wasm/wasm-objects.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/wasm/wasm-objects.h"
 #include "src/utils.h"
 
 #include "src/assembler-inl.h"
 #include "src/base/iterator.h"
 #include "src/compiler/wasm-compiler.h"
@@ skipping 311 matching lines @@
       isolate->factory()->NewJSObject(memory_ctor, TENURED));
   if (buffer.is_null()) {
     const bool enable_guard_regions = EnableGuardRegions();
     buffer = SetupArrayBuffer(isolate, nullptr, 0, nullptr, 0, false,
                               enable_guard_regions);
   }
   memory_obj->set_array_buffer(*buffer);
   memory_obj->set_maximum_pages(maximum);
   Handle<Symbol> memory_sym(isolate->native_context()->wasm_memory_sym());
   Object::SetProperty(memory_obj, memory_sym, memory_obj, STRICT).Check();
-  return Handle<WasmMemoryObject>::cast(memory_obj);
+
+  return memory_obj;
 }
 
 uint32_t WasmMemoryObject::current_pages() {
   uint32_t byte_length;
   CHECK(array_buffer()->byte_length()->ToUint32(&byte_length));
   return byte_length / wasm::WasmModule::kPageSize;
 }
 
 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);

[Clemens Hammacher, 2017/07/07 11:56:21]

This is now linear in the number of instances, instead of constant. I think you
should at least mention this in the CL description.

[titzer, 2017/07/07 12:07:41]

Add a comment to the declaration.

   }
-  set_instances_link(*instance_wrapper);
 }
 
 // static
 int32_t WasmMemoryObject::Grow(Isolate* isolate,
                                Handle<WasmMemoryObject> memory_object,
                                uint32_t pages) {
   Handle<JSArrayBuffer> old_buffer(memory_object->array_buffer());
   uint32_t old_size = 0;
   CHECK(old_buffer->byte_length()->ToUint32(&old_size));
   Handle<JSArrayBuffer> new_buffer;
   // Return current size if grow by 0.
   if (pages == 0) {
     // Even for pages == 0, we need to attach a new JSArrayBuffer with the same
     // backing store and neuter the old one to be spec compliant.
     if (old_size != 0) {
       new_buffer = SetupArrayBuffer(
           isolate, old_buffer->allocation_base(),
           old_buffer->allocation_length(), old_buffer->backing_store(),
           old_size, old_buffer->is_external(), old_buffer->has_guard_region());
       memory_object->set_array_buffer(*new_buffer);
     }
     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;

[Clemens Hammacher, 2017/07/07 11:56:21]

This code is much clearer now 👍
Hard to see if it's semantically equivalent (because the old code was a mess),
so let's hope that we have enough test coverage here.

[titzer, 2017/07/07 12:07:41]

Acknowledged.

+  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();
+    max_pages = FLAG_wasm_max_mem_pages;
+  }
+  new_buffer = GrowMemoryBuffer(isolate, old_buffer, pages, max_pages);
+  if (new_buffer.is_null()) return -1;
 
-    // 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);
+  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;
 }
 
 WasmModuleObject* WasmInstanceObject::module_object() {
   return *compiled_module()->wasm_module();
 }
 
 WasmModule* WasmInstanceObject::module() { return compiled_module()->module(); }
@@ skipping 13 matching lines @@
   Handle<JSObject> instance_object =
       isolate->factory()->NewJSObject(instance_cons, TENURED);
 
   Handle<Symbol> instance_sym(isolate->native_context()->wasm_instance_sym());
   Object::SetProperty(instance_object, instance_sym, instance_object, STRICT)
       .Check();
   Handle<WasmInstanceObject> instance(
       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;
 }
 
 int32_t WasmInstanceObject::GetMemorySize() {
   if (!has_memory_buffer()) return 0;
   uint32_t bytes = memory_buffer()->byte_length()->Number();
   DCHECK_EQ(0, bytes % WasmModule::kPageSize);
   return bytes / WasmModule::kPageSize;
 }
 
@@ skipping 935 matching lines @@
     Isolate* isolate, Handle<WasmInstanceObject> instance, Handle<Code> caller,
     int offset, int func_index, bool patch_caller) {
   isolate->set_context(*instance->compiled_module()->native_context());
   Object* orch_obj =
       instance->compiled_module()->shared()->lazy_compilation_orchestrator();
   LazyCompilationOrchestrator* orch =
       Managed<LazyCompilationOrchestrator>::cast(orch_obj)->get();
   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;
-}