Tell the GC prologues to construct RetainedObjectInfos only when needed. (issue 14471028)

6 views
Skip to first unread message

ma...@chromium.org

unread,
Apr 25, 2013, 5:36:22 AM4/25/13
to hpa...@chromium.org, v8-...@googlegroups.com, mstar...@chromium.org
Reviewers: Hannes Payer,

Message:
hpayer, pls review.

mstarzinger ACKed this approach, but we didn't discuss naming the flag.

Description:
Tell the GC prologues to construct RetainedObjectInfos only when needed.

The GC prologue is called by the GC, but also by the heap snapshotter. The
RetainedObjectInfos are only needed by the heap snapshotter, so it's
wasteful to
construct them always. (And it will be even more wasteful when Blink
migrates to
the new GC APIs, since after that point it no longer knows about object
groups.)

BUG=

Please review this at https://codereview.chromium.org/14471028/

SVN Base: git://github.com/v8/v8.git@master

Affected files:
M include/v8.h
M src/heap-snapshot-generator.cc
M src/heap.h
M src/heap.cc


Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index
e906bd9e2b1ec63e0ec15067f95ed3fe9bec74cc..1a7fb031821bd08d07d916b24294734b867e4816
100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -3016,7 +3016,8 @@ enum GCType {

enum GCCallbackFlags {
kNoGCCallbackFlags = 0,
- kGCCallbackFlagCompacted = 1 << 0
+ kGCCallbackFlagCompacted = 1 << 0,
+ kGCCallbackFlagConstructRetainedObjectInfos = 1 << 1
};

typedef void (*GCPrologueCallback)(GCType type, GCCallbackFlags flags);
Index: src/heap-snapshot-generator.cc
diff --git a/src/heap-snapshot-generator.cc b/src/heap-snapshot-generator.cc
index
bbb46a1892ff3456756569f68b04d647d7742bc4..3d890f720ec5aeb19d7620d7a3b7859dc27b098f
100644
--- a/src/heap-snapshot-generator.cc
+++ b/src/heap-snapshot-generator.cc
@@ -1936,7 +1936,8 @@ void NativeObjectsExplorer::FillRetainedObjects() {
Isolate* isolate = Isolate::Current();
const GCType major_gc_type = kGCTypeMarkSweepCompact;
// Record objects that are joined into ObjectGroups.
- isolate->heap()->CallGCPrologueCallbacks(major_gc_type);
+ isolate->heap()->CallGCPrologueCallbacks(
+ major_gc_type, kGCCallbackFlagConstructRetainedObjectInfos);
List<ObjectGroup*>* groups = isolate->global_handles()->object_groups();
for (int i = 0; i < groups->length(); ++i) {
ObjectGroup* group = groups->at(i);
Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index
a1e2a48c0576afa7c86751fe327b9f3d550ecc8c..7976cf8aa7f1a4965dc844c21d4254f07580a8df
100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -893,7 +893,7 @@ bool Heap::PerformGarbageCollection(GarbageCollector
collector,
{
GCTracer::Scope scope(tracer, GCTracer::Scope::EXTERNAL);
VMState<EXTERNAL> state(isolate_);
- CallGCPrologueCallbacks(gc_type);
+ CallGCPrologueCallbacks(gc_type, kNoGCCallbackFlags);
}

EnsureFromSpaceIsCommitted();
@@ -1028,13 +1028,13 @@ bool
Heap::PerformGarbageCollection(GarbageCollector collector,
}


-void Heap::CallGCPrologueCallbacks(GCType gc_type) {
+void Heap::CallGCPrologueCallbacks(GCType gc_type, GCCallbackFlags flags) {
if (gc_type == kGCTypeMarkSweepCompact && global_gc_prologue_callback_) {
global_gc_prologue_callback_();
}
for (int i = 0; i < gc_prologue_callbacks_.length(); ++i) {
if (gc_type & gc_prologue_callbacks_[i].gc_type) {
- gc_prologue_callbacks_[i].callback(gc_type, kNoGCCallbackFlags);
+ gc_prologue_callbacks_[i].callback(gc_type, flags);
}
}
}
Index: src/heap.h
diff --git a/src/heap.h b/src/heap.h
index
4cd63442f1c60d30e982c57baee9d705c531ed8c..e3406aec1f1e0444e61937b11cc1087c0da5c810
100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -1755,7 +1755,7 @@ class Heap {

inline Isolate* isolate();

- void CallGCPrologueCallbacks(GCType gc_type);
+ void CallGCPrologueCallbacks(GCType gc_type, GCCallbackFlags flags);
void CallGCEpilogueCallbacks(GCType gc_type);

inline bool OldGenerationAllocationLimitReached();


hpa...@chromium.org

unread,
Apr 25, 2013, 7:47:11 AM4/25/13
to ma...@chromium.org, v8-...@googlegroups.com, mstar...@chromium.org

hpa...@chromium.org

unread,
Apr 25, 2013, 8:08:17 AM4/25/13
to ma...@chromium.org, v8-...@googlegroups.com, mstar...@chromium.org
Committed patchset #1 manually as r14439 (presubmit successful).

https://codereview.chromium.org/14471028/
Reply all
Reply to author
Forward
0 new messages