Reviewers: Hannes Payer,
Message:
PTAL
Description:
Reload length of retained_maps array after GC.
This fixes flaky GC stress failure:
BUG=
TEST=test-heap/RegressArrayListGC
Please review this at https://codereview.chromium.org/1026113004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+55, -13 lines):
M src/heap/heap.cc
M src/heap/mark-compact.cc
M src/objects.h
M src/objects.cc
M test/cctest/test-heap.cc
Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index
3f652531ecd77f21bb3b7942e8d530f92d66b56a..2ac5146c46d0cf06c6f00f2bbea878a0015db66b
100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -5613,7 +5613,8 @@ void Heap::AddRetainedMap(Handle<Map> map) {
Handle<WeakCell> cell = Map::WeakCellForMap(map);
Handle<ArrayList> array(retained_maps(), isolate());
array = ArrayList::Add(
- array, cell, handle(Smi::FromInt(FLAG_retain_maps_for_n_gc),
isolate()));
+ array, cell, handle(Smi::FromInt(FLAG_retain_maps_for_n_gc),
isolate()),
+ ArrayList::kReloadLengthAfterAllocation);
if (*array != retained_maps()) {
set_retained_maps(*array);
}
Index: src/heap/mark-compact.cc
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index
de381aab356657374f1e62f083173045d949ded0..e6b8e706172702104166f46b5f8ff4121a07cb57
100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -2172,6 +2172,7 @@ void MarkCompactCollector::RetainMaps() {
retained_maps->Clear(i, undefined);
}
if (new_length != length) retained_maps->SetLength(new_length);
+ printf("new %d, old %d\n", new_length, length);
ProcessMarkingDeque();
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
f7d0fd5b65fc5fa923c7dfbccf9cda42128a6795..12184a81df6599c3452eb3101e35179054c27a67
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -8277,9 +8277,13 @@ Handle<WeakFixedArray> WeakFixedArray::Allocate(
}
-Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object>
obj) {
+Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object>
obj,
+ AddMode mode) {
int length = array->Length();
array = EnsureSpace(array, length + 1);
+ if (mode == kReloadLengthAfterAllocation) {
+ length = array->Length();
+ }
array->Set(length, *obj);
array->SetLength(length + 1);
return array;
@@ -8287,9 +8291,12 @@ Handle<ArrayList> ArrayList::Add(Handle<ArrayList>
array, Handle<Object> obj) {
Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object>
obj1,
- Handle<Object> obj2) {
+ Handle<Object> obj2, AddMode mode) {
int length = array->Length();
array = EnsureSpace(array, length + 2);
+ if (mode == kReloadLengthAfterAllocation) {
+ length = array->Length();
+ }
array->Set(length, *obj1);
array->Set(length + 1, *obj2);
array->SetLength(length + 2);
@@ -8299,10 +8306,12 @@ Handle<ArrayList> ArrayList::Add(Handle<ArrayList>
array, Handle<Object> obj1,
Handle<ArrayList> ArrayList::EnsureSpace(Handle<ArrayList> array, int
length) {
int capacity = array->length();
+ bool empty = (capacity == 0);
if (capacity < kFirstIndex + length) {
capacity = kFirstIndex + length;
capacity = capacity + Max(capacity / 2, 2);
array = Handle<ArrayList>::cast(FixedArray::CopySize(array, capacity));
+ if (empty) array->SetLength(0);
}
return array;
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
5961dc060f82ac581ccda19073d2375e2dfe9b34..db40084573f5d52adfb602999bf955602dc92ddd
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2631,9 +2631,15 @@ class WeakFixedArray : public FixedArray {
// Generic array grows dynamically with O(1) amortized insertion.
class ArrayList : public FixedArray {
public:
- static Handle<ArrayList> Add(Handle<ArrayList> array, Handle<Object>
obj);
+ enum AddMode {
+ kNone,
+ // Use this if GC can delete elements from array.
+ kReloadLengthAfterAllocation,
+ };
+ static Handle<ArrayList> Add(Handle<ArrayList> array, Handle<Object> obj,
+ AddMode mode = kNone);
static Handle<ArrayList> Add(Handle<ArrayList> array, Handle<Object>
obj1,
- Handle<Object> obj2);
+ Handle<Object> obj2, AddMode = kNone);
inline int Length();
inline void SetLength(int length);
inline Object* Get(int index);
Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index
f5d71c37fccb0788df53f86d134f7b60ec94d64e..8dfa2caae8f6646dcc2ecf58943d6683f8647723
100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -5096,12 +5096,7 @@ TEST(Regress3877) {
}
-void CheckMapRetainingFor(int n) {
- FLAG_retain_maps_for_n_gc = n;
- Isolate* isolate = CcTest::i_isolate();
- Heap* heap = isolate->heap();
- Handle<WeakCell> weak_cell;
- {
+Handle<WeakCell> AddRetainedMap(Isolate* isolate, Heap* heap) {
HandleScope inner_scope(isolate);
Handle<Map> map = Map::Create(isolate, 1);
v8::Local<v8::Value> result =
@@ -5110,8 +5105,15 @@ void CheckMapRetainingFor(int n) {
v8::Utils::OpenHandle(*v8::Handle<v8::Object>::Cast(result));
map->set_prototype(*proto);
heap->AddRetainedMap(map);
- weak_cell = inner_scope.CloseAndEscape(Map::WeakCellForMap(map));
- }
+ return inner_scope.CloseAndEscape(Map::WeakCellForMap(map));
+}
+
+
+void CheckMapRetainingFor(int n) {
+ FLAG_retain_maps_for_n_gc = n;
+ Isolate* isolate = CcTest::i_isolate();
+ Heap* heap = isolate->heap();
+ Handle<WeakCell> weak_cell = AddRetainedMap(isolate, heap);
CHECK(!weak_cell->cleared());
for (int i = 0; i < n; i++) {
heap->CollectGarbage(OLD_POINTER_SPACE);
@@ -5132,6 +5134,29 @@ TEST(MapRetaining) {
}
+TEST(RegressArrayListGC) {
+ FLAG_retain_maps_for_n_gc = 1;
+ FLAG_incremental_marking = 0;
+ FLAG_gc_global = true;
+ CcTest::InitializeVM();
+ v8::HandleScope scope(CcTest::isolate());
+ Isolate* isolate = CcTest::i_isolate();
+ Heap* heap = isolate->heap();
+ Handle<WeakCell> weak_cell = AddRetainedMap(isolate, heap);
+ Handle<Map> map = Map::Create(isolate, 1);
+ DCHECK(!weak_cell->cleared());
+ heap->CollectGarbage(OLD_POINTER_SPACE);
+ DCHECK(!weak_cell->cleared());
+ // Force GC in old space on next addition of retained map.
+ Map::WeakCellForMap(map);
+ SimulateFullSpace(CcTest::heap()->new_space());
+ for (int i = 0; i < 10; i++) {
+ heap->AddRetainedMap(map);
+ }
+ heap->CollectGarbage(OLD_POINTER_SPACE);
+}
+
+
#ifdef DEBUG
TEST(PathTracer) {
CcTest::InitializeVM();
--
--
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.
