Reviewers: Hannes Payer, Michael Starzinger,
Description:
After moving unreachable weak global handles only process harmony
collections
Groups and implicit references are no longer relevant at this point.
Also add tests that fail if the first or second round of ephemeral
marking is omitted
BUG=none
[email protected],[email protected]
LOG=n
Please review this at https://codereview.chromium.org/761343004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+66, -24 lines):
M src/heap/mark-compact.h
M src/heap/mark-compact.cc
M test/cctest/test-api.cc
M test/cctest/test-weakmaps.cc
Index: src/heap/mark-compact.cc
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index
847bdac1263db4c9de13aa624e934bb5edcbcdba..c2540a63e89549434a4392275aef034dbfcbed81
100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -2078,13 +2078,16 @@ void MarkCompactCollector::ProcessMarkingDeque() {
// Mark all objects reachable (transitively) from objects on the marking
// stack including references only considered in the atomic marking pause.
-void MarkCompactCollector::ProcessEphemeralMarking(ObjectVisitor* visitor)
{
+void MarkCompactCollector::ProcessEphemeralMarking(
+ ObjectVisitor* visitor, bool only_process_harmony_weak_collections) {
bool work_to_do = true;
DCHECK(marking_deque_.IsEmpty());
while (work_to_do) {
- isolate()->global_handles()->IterateObjectGroups(
- visitor, &IsUnmarkedHeapObjectWithHeap);
- MarkImplicitRefGroups();
+ if (!only_process_harmony_weak_collections) {
+ isolate()->global_handles()->IterateObjectGroups(
+ visitor, &IsUnmarkedHeapObjectWithHeap);
+ MarkImplicitRefGroups();
+ }
ProcessWeakCollections();
work_to_do = !marking_deque_.IsEmpty();
ProcessMarkingDeque();
@@ -2196,26 +2199,24 @@ void MarkCompactCollector::MarkLiveObjects() {
// The objects reachable from the roots are marked, yet unreachable
// objects are unmarked. Mark objects reachable due to host
// application specific logic or through Harmony weak maps.
- ProcessEphemeralMarking(&root_visitor);
+ ProcessEphemeralMarking(&root_visitor, false);
// The objects reachable from the roots, weak maps or object groups
- // are marked, yet unreachable objects are unmarked. Mark objects
+ // are marked. Objects pointed to only by weak global handles cannot be
+ // immediately reclaimed. Instead, we have to mark them as pending and
mark
+ // objects reachable from them.
// reachable only from weak global handles.
//
// First we identify nonlive weak handles and mark them as pending
// destruction.
heap()->isolate()->global_handles()->IdentifyWeakHandles(
&IsUnmarkedHeapObject);
- // Then we mark the objects and process the transitive closure.
+ // Then we mark the objects.
heap()->isolate()->global_handles()->IterateWeakRoots(&root_visitor);
- while (marking_deque_.overflowed()) {
- RefillMarkingDeque();
- EmptyMarkingDeque();
- }
- // Repeat host application specific and Harmony weak maps marking to
- // mark unmarked objects reachable from the weak roots.
- ProcessEphemeralMarking(&root_visitor);
+ // Repeat Harmony weak maps marking to mark unmarked objects reachable
from
+ // the weak roots we just marked as pending destruction.
+ ProcessEphemeralMarking(&root_visitor, false);
AfterMarking();
Index: src/heap/mark-compact.h
diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h
index
b1ad5b8a80ee6547e9a03708a473a46c913822ce..868714a97d56528938f7bdd8d2a3841a888f622e
100644
--- a/src/heap/mark-compact.h
+++ b/src/heap/mark-compact.h
@@ -764,7 +764,8 @@ class MarkCompactCollector {
// - Processing of objects reachable through Harmony WeakMaps.
// - Objects reachable due to host application logic like object
groups
// or implicit references' groups.
- void ProcessEphemeralMarking(ObjectVisitor* visitor);
+ void ProcessEphemeralMarking(ObjectVisitor* visitor,
+ bool only_process_harmony_weak_collections);
// If the call-site of the top optimized code was not prepared for
// deoptimization, then treat the maps in the code as strong pointers,
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
0920178785e1664112f32c7c375c8c306ba7df64..3f141fe471e1848b3edec55f57fedf4e55f9ea57
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -4383,6 +4383,45 @@ THREADED_TEST(ApiObjectGroupsCycle) {
}
+THREADED_TEST(WeakRootsSurviveTwoRoundsOfGC) {
+ LocalContext env;
+ v8::Isolate* iso = env->GetIsolate();
+ HandleScope scope(iso);
+
+ WeakCallCounter counter(1234);
+
+ WeakCallCounterAndPersistent<Value> weak_obj(&counter);
+
+ // Create a weak object that references a internalized string.
+ {
+ HandleScope scope(iso);
+ weak_obj.handle.Reset(iso, Object::New(iso));
+ weak_obj.handle.SetWeak(&weak_obj, &WeakPointerCallback);
+ CHECK(weak_obj.handle.IsWeak());
+ Local<Object>::New(iso, weak_obj.handle.As<Object>())->Set(
+ v8_str("x"),
+ String::NewFromUtf8(iso, "magic cookie",
String::kInternalizedString));
+ }
+ // Do a single full GC
+ i::Isolate* i_iso = reinterpret_cast<v8::internal::Isolate*>(iso);
+ i::Heap* heap = i_iso->heap();
+ heap->CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
+
+ // We should have received the weak callback.
+ CHECK_EQ(1, counter.NumberOfWeakCalls());
+
+ // Check that the string is still alive.
+ {
+ HandleScope scope(iso);
+ i::MaybeHandle<i::String> magic_string =
+ i::StringTable::LookupStringIfExists(
+ i_iso,
+ v8::Utils::OpenHandle(*String::NewFromUtf8(iso, "magic
cookie")));
+ magic_string.Check();
+ }
+}
+
+
// TODO(mstarzinger): This should be a THREADED_TEST but causes failures
// on the buildbots, so was made non-threaded for the time being.
TEST(ApiObjectGroupsCycleForScavenger) {
Index: test/cctest/test-weakmaps.cc
diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc
index
bb412a82aadcf675f41794634710ab37409f7c96..6c19cb8e47d5caa1c92b96d9704e23653424b9e5
100644
--- a/test/cctest/test-weakmaps.cc
+++ b/test/cctest/test-weakmaps.cc
@@ -97,19 +97,20 @@ TEST(Weakness) {
}
CHECK(!global_handles->IsWeak(key.location()));
- // Put entry into weak map.
+ // Put two chained entries into weak map.
{
HandleScope scope(isolate);
- PutIntoWeakMap(weakmap,
- Handle<JSObject>(JSObject::cast(*key)),
- Handle<Smi>(Smi::FromInt(23), isolate));
+ Handle<Map> map = factory->NewMap(JS_OBJECT_TYPE,
JSObject::kHeaderSize);
+ Handle<JSObject> object = factory->NewJSObjectFromMap(map);
+ PutIntoWeakMap(weakmap, Handle<JSObject>(JSObject::cast(*key)),
object);
+ PutIntoWeakMap(weakmap, object, Handle<Smi>(Smi::FromInt(23),
isolate));
}
- CHECK_EQ(1, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
+ CHECK_EQ(2, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
// Force a full GC.
heap->CollectAllGarbage(false);
CHECK_EQ(0, NumberOfWeakCalls);
- CHECK_EQ(1, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
+ CHECK_EQ(2, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
CHECK_EQ(
0,
ObjectHashTable::cast(weakmap->table())->NumberOfDeletedElements());
@@ -128,14 +129,14 @@ TEST(Weakness) {
// weak references whereas the second one will also clear weak maps.
heap->CollectAllGarbage(false);
CHECK_EQ(1, NumberOfWeakCalls);
- CHECK_EQ(1, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
+ CHECK_EQ(2, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
CHECK_EQ(
0,
ObjectHashTable::cast(weakmap->table())->NumberOfDeletedElements());
heap->CollectAllGarbage(false);
CHECK_EQ(1, NumberOfWeakCalls);
CHECK_EQ(0, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
- CHECK_EQ(
- 1,
ObjectHashTable::cast(weakmap->table())->NumberOfDeletedElements());
+ CHECK_EQ(2,
+
ObjectHashTable::cast(weakmap->table())->NumberOfDeletedElements());
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
