Revision: 7521
Author: [email protected]
Date: Wed Apr 6 12:17:54 2011
Log: Make object groups and implicit references a bit more lightweight.
We can only call malloc/free once per group and we can avoid scanning
through a list of NULLs if we keep unprocessed groups in the beginning.
I also changed the internal representation of implicit references to
hold a handle to the parent (instead of a direct pointer). The
prologue callback must not trigger a GC, but it's better to be safe.
Review URL: http://codereview.chromium.org/6800003
http://code.google.com/p/v8/source/detail?r=7521
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/global-handles.cc
/branches/bleeding_edge/src/global-handles.h
/branches/bleeding_edge/src/mark-compact.cc
/branches/bleeding_edge/src/profile-generator.cc
/branches/bleeding_edge/src/v8utils.h
/branches/bleeding_edge/test/cctest/test-mark-compact.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Fri Apr 1 05:17:20 2011
+++ /branches/bleeding_edge/src/api.cc Wed Apr 6 12:17:54 2011
@@ -4473,7 +4473,7 @@
if (IsDeadCheck(isolate, "v8::V8::AddImplicitReferences()")) return;
STATIC_ASSERT(sizeof(Persistent<Value>) == sizeof(i::Object**));
isolate->global_handles()->AddImplicitReferences(
- *Utils::OpenHandle(*parent),
+
i::Handle<i::HeapObject>::cast(Utils::OpenHandle(*parent)).location(),
reinterpret_cast<i::Object***>(children), length);
}
=======================================
--- /branches/bleeding_edge/src/global-handles.cc Fri Mar 18 13:35:07 2011
+++ /branches/bleeding_edge/src/global-handles.cc Wed Apr 6 12:17:54 2011
@@ -558,28 +558,25 @@
void GlobalHandles::AddObjectGroup(Object*** handles,
size_t length,
v8::RetainedObjectInfo* info) {
- ObjectGroup* new_entry = new ObjectGroup(length, info);
- for (size_t i = 0; i < length; ++i) {
- new_entry->objects_.Add(handles[i]);
- }
- object_groups_.Add(new_entry);
+ if (length == 0) {
+ if (info != NULL) info->Dispose();
+ return;
+ }
+ object_groups_.Add(ObjectGroup::New(handles, length, info));
}
-void GlobalHandles::AddImplicitReferences(HeapObject* parent,
+void GlobalHandles::AddImplicitReferences(HeapObject** parent,
Object*** children,
size_t length) {
- ImplicitRefGroup* new_entry = new ImplicitRefGroup(parent, length);
- for (size_t i = 0; i < length; ++i) {
- new_entry->children_.Add(children[i]);
- }
- implicit_ref_groups_.Add(new_entry);
+ if (length == 0) return;
+ implicit_ref_groups_.Add(ImplicitRefGroup::New(parent, children,
length));
}
void GlobalHandles::RemoveObjectGroups() {
for (int i = 0; i < object_groups_.length(); i++) {
- delete object_groups_.at(i);
+ object_groups_.at(i)->Dispose();
}
object_groups_.Clear();
}
@@ -587,7 +584,7 @@
void GlobalHandles::RemoveImplicitRefGroups() {
for (int i = 0; i < implicit_ref_groups_.length(); i++) {
- delete implicit_ref_groups_.at(i);
+ implicit_ref_groups_.at(i)->Dispose();
}
implicit_ref_groups_.Clear();
}
=======================================
--- /branches/bleeding_edge/src/global-handles.h Fri Mar 18 13:35:07 2011
+++ /branches/bleeding_edge/src/global-handles.h Wed Apr 6 12:17:54 2011
@@ -42,37 +42,66 @@
// An object group is treated like a single JS object: if one of object in
// the group is alive, all objects in the same group are considered alive.
// An object group is used to simulate object relationship in a DOM tree.
-class ObjectGroup : public Malloced {
+class ObjectGroup {
public:
- ObjectGroup() : objects_(4) {}
- ObjectGroup(size_t capacity, v8::RetainedObjectInfo* info)
- : objects_(static_cast<int>(capacity)),
- info_(info) { }
- ~ObjectGroup();
-
- List<Object**> objects_;
+ static ObjectGroup* New(Object*** handles,
+ size_t length,
+ v8::RetainedObjectInfo* info) {
+ ASSERT(length > 0);
+ ObjectGroup* group = reinterpret_cast<ObjectGroup*>(
+ malloc(OFFSET_OF(ObjectGroup, objects_[length])));
+ group->length_ = length;
+ group->info_ = info;
+ CopyWords(group->objects_, handles, length);
+ return group;
+ }
+
+ void Dispose() {
+ free(this);
+ }
+
+ size_t length_;
v8::RetainedObjectInfo* info_;
+ Object** objects_[1]; // Variable sized array.
private:
- DISALLOW_COPY_AND_ASSIGN(ObjectGroup);
+ void* operator new(size_t size);
+ void operator delete(void* p);
+ ~ObjectGroup();
+ DISALLOW_IMPLICIT_CONSTRUCTORS(ObjectGroup);
};
// An implicit references group consists of two parts: a parent object and
// a list of children objects. If the parent is alive, all the children
// are alive too.
-class ImplicitRefGroup : public Malloced {
+class ImplicitRefGroup {
public:
- ImplicitRefGroup() : children_(4) {}
- ImplicitRefGroup(HeapObject* parent, size_t capacity)
- : parent_(parent),
- children_(static_cast<int>(capacity)) { }
-
- HeapObject* parent_;
- List<Object**> children_;
+ static ImplicitRefGroup* New(HeapObject** parent,
+ Object*** children,
+ size_t length) {
+ ASSERT(length > 0);
+ ImplicitRefGroup* group = reinterpret_cast<ImplicitRefGroup*>(
+ malloc(OFFSET_OF(ImplicitRefGroup, children_[length])));
+ group->parent_ = parent;
+ group->length_ = length;
+ CopyWords(group->children_, children, length);
+ return group;
+ }
+
+ void Dispose() {
+ free(this);
+ }
+
+ HeapObject** parent_;
+ size_t length_;
+ Object** children_[1]; // Variable sized array.
private:
- DISALLOW_COPY_AND_ASSIGN(ImplicitRefGroup);
+ void* operator new(size_t size);
+ void operator delete(void* p);
+ ~ImplicitRefGroup();
+ DISALLOW_IMPLICIT_CONSTRUCTORS(ImplicitRefGroup);
};
@@ -154,7 +183,7 @@
// Add an implicit references' group.
// Should be only used in GC callback function before a collection.
// All groups are destroyed after a mark-compact collection.
- void AddImplicitReferences(HeapObject* parent,
+ void AddImplicitReferences(HeapObject** parent,
Object*** children,
size_t length);
=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Tue Apr 5 02:01:47 2011
+++ /branches/bleeding_edge/src/mark-compact.cc Wed Apr 6 12:17:54 2011
@@ -1217,13 +1217,14 @@
List<ObjectGroup*>* object_groups =
heap()->isolate()->global_handles()->object_groups();
+ int last = 0;
for (int i = 0; i < object_groups->length(); i++) {
ObjectGroup* entry = object_groups->at(i);
- if (entry == NULL) continue;
-
- List<Object**>& objects = entry->objects_;
+ ASSERT(entry != NULL);
+
+ Object*** objects = entry->objects_;
bool group_marked = false;
- for (int j = 0; j < objects.length(); j++) {
+ for (size_t j = 0; j < entry->length_; j++) {
Object* object = *objects[j];
if (object->IsHeapObject() && HeapObject::cast(object)->IsMarked()) {
group_marked = true;
@@ -1231,21 +1232,24 @@
}
}
- if (!group_marked) continue;
-
- // An object in the group is marked, so mark as gray all white heap
- // objects in the group.
- for (int j = 0; j < objects.length(); ++j) {
+ if (!group_marked) {
+ (*object_groups)[last++] = entry;
+ continue;
+ }
+
+ // An object in the group is marked, so mark all heap objects in
+ // the group.
+ for (size_t j = 0; j < entry->length_; ++j) {
if ((*objects[j])->IsHeapObject()) {
MarkObject(HeapObject::cast(*objects[j]));
}
}
- // Once the entire group has been colored gray, set the object group
- // to NULL so it won't be processed again.
- delete entry;
- object_groups->at(i) = NULL;
- }
+ // Once the entire group has been marked, dispose it because it's
+ // not needed anymore.
+ entry->Dispose();
+ }
+ object_groups->Rewind(last);
}
@@ -1253,26 +1257,29 @@
List<ImplicitRefGroup*>* ref_groups =
heap()->isolate()->global_handles()->implicit_ref_groups();
+ int last = 0;
for (int i = 0; i < ref_groups->length(); i++) {
ImplicitRefGroup* entry = ref_groups->at(i);
- if (entry == NULL) continue;
-
- if (!entry->parent_->IsMarked()) continue;
-
- List<Object**>& children = entry->children_;
- // A parent object is marked, so mark as gray all child white heap
- // objects.
- for (int j = 0; j < children.length(); ++j) {
+ ASSERT(entry != NULL);
+
+ if (!(*entry->parent_)->IsMarked()) {
+ (*ref_groups)[last++] = entry;
+ continue;
+ }
+
+ Object*** children = entry->children_;
+ // A parent object is marked, so mark all child heap objects.
+ for (size_t j = 0; j < entry->length_; ++j) {
if ((*children[j])->IsHeapObject()) {
MarkObject(HeapObject::cast(*children[j]));
}
}
- // Once the entire group has been colored gray, set the group
- // to NULL so it won't be processed again.
- delete entry;
- ref_groups->at(i) = NULL;
- }
+ // Once the entire group has been marked, dispose it because it's
+ // not needed anymore.
+ entry->Dispose();
+ }
+ ref_groups->Rewind(last);
}
=======================================
--- /branches/bleeding_edge/src/profile-generator.cc Wed Mar 30 07:04:50
2011
+++ /branches/bleeding_edge/src/profile-generator.cc Wed Apr 6 12:17:54
2011
@@ -2295,7 +2295,7 @@
ObjectGroup* group = groups->at(i);
if (group->info_ == NULL) continue;
List<HeapObject*>* list = GetListMaybeDisposeInfo(group->info_);
- for (int j = 0; j < group->objects_.length(); ++j) {
+ for (size_t j = 0; j < group->length_; ++j) {
HeapObject* obj = HeapObject::cast(*group->objects_[j]);
list->Add(obj);
in_groups_.Insert(obj);
=======================================
--- /branches/bleeding_edge/src/v8utils.h Wed Mar 30 07:04:26 2011
+++ /branches/bleeding_edge/src/v8utils.h Wed Apr 6 12:17:54 2011
@@ -120,7 +120,9 @@
// Memory
// Copies data from |src| to |dst|. The data spans MUST not overlap.
-inline void CopyWords(Object** dst, Object** src, int num_words) {
+template <typename T>
+inline void CopyWords(T* dst, T* src, int num_words) {
+ STATIC_ASSERT(sizeof(T) == kPointerSize);
ASSERT(Min(dst, src) + num_words <= Max(dst, src));
ASSERT(num_words > 0);
=======================================
--- /branches/bleeding_edge/test/cctest/test-mark-compact.cc Fri Mar 18
13:35:07 2011
+++ /branches/bleeding_edge/test/cctest/test-mark-compact.cc Wed Apr 6
12:17:54 2011
@@ -299,8 +299,8 @@
}
TEST(ObjectGroups) {
- GlobalHandles* global_handles = Isolate::Current()->global_handles();
InitializeVM();
+ GlobalHandles* global_handles = Isolate::Current()->global_handles();
NumberOfWeakCalls = 0;
v8::HandleScope handle_scope;
@@ -308,9 +308,9 @@
Handle<Object> g1s1 =
global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
Handle<Object> g1s2 =
- global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
Handle<Object> g1c1 =
- global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
global_handles->MakeWeak(g1s1.location(),
reinterpret_cast<void*>(1234),
&WeakPointerCallback);
@@ -349,11 +349,11 @@
Object** g2_objects[] = { g2s1.location(), g2s2.location() };
Object** g2_children[] = { g2c1.location() };
global_handles->AddObjectGroup(g1_objects, 2, NULL);
- global_handles->AddImplicitReferences(HeapObject::cast(*g1s1),
- g1_children, 1);
+ global_handles->AddImplicitReferences(
+ Handle<HeapObject>::cast(g1s1).location(), g1_children, 1);
global_handles->AddObjectGroup(g2_objects, 2, NULL);
- global_handles->AddImplicitReferences(HeapObject::cast(*g2s2),
- g2_children, 1);
+ global_handles->AddImplicitReferences(
+ Handle<HeapObject>::cast(g2s2).location(), g2_children, 1);
}
// Do a full GC
HEAP->CollectGarbage(OLD_POINTER_SPACE);
@@ -377,11 +377,11 @@
Object** g2_objects[] = { g2s1.location(), g2s2.location() };
Object** g2_children[] = { g2c1.location() };
global_handles->AddObjectGroup(g1_objects, 2, NULL);
- global_handles->AddImplicitReferences(HeapObject::cast(*g1s1),
- g1_children, 1);
+ global_handles->AddImplicitReferences(
+ Handle<HeapObject>::cast(g1s1).location(), g1_children, 1);
global_handles->AddObjectGroup(g2_objects, 2, NULL);
- global_handles->AddImplicitReferences(HeapObject::cast(*g2s2),
- g2_children, 1);
+ global_handles->AddImplicitReferences(
+ Handle<HeapObject>::cast(g2s2).location(), g2_children, 1);
}
HEAP->CollectGarbage(OLD_POINTER_SPACE);
@@ -400,3 +400,45 @@
HEAP->CollectGarbage(OLD_POINTER_SPACE);
CHECK_EQ(7, NumberOfWeakCalls);
}
+
+
+class TestRetainedObjectInfo : public v8::RetainedObjectInfo {
+ public:
+ TestRetainedObjectInfo() : has_been_disposed_(false) {}
+
+ bool has_been_disposed() { return has_been_disposed_; }
+
+ virtual void Dispose() {
+ ASSERT(!has_been_disposed_);
+ has_been_disposed_ = true;
+ }
+
+ virtual bool IsEquivalent(v8::RetainedObjectInfo* other) {
+ return other == this;
+ }
+
+ virtual intptr_t GetHash() { return 0; }
+
+ virtual const char* GetLabel() { return "whatever"; }
+
+ private:
+ bool has_been_disposed_;
+};
+
+
+TEST(EmptyObjectGroups) {
+ InitializeVM();
+ GlobalHandles* global_handles = Isolate::Current()->global_handles();
+
+ v8::HandleScope handle_scope;
+
+ Handle<Object> object =
+
global_handles->Create(HEAP->AllocateFixedArray(1)->ToObjectChecked());
+
+ TestRetainedObjectInfo info;
+ global_handles->AddObjectGroup(NULL, 0, &info);
+ ASSERT(info.has_been_disposed());
+
+ global_handles->AddImplicitReferences(
+ Handle<HeapObject>::cast(object).location(), NULL, 0);
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
