Commit 89f5bf7f authored by Marja Hölttä's avatar Marja Hölttä Committed by Commit Bot

[heap] Remove unnecessary length reloading from ArrayList::Add.

The reloading was needed when GC would compact the Heap::retained_maps
array. But that's no longer true; the compaction is done in
Heap::AddRetainedMap, outside GC. So it's not possible that the length would
change because of an allocation.

(Pre-cleanup for in-place weak ref work.)

BUG=v8:7308

Change-Id: I18554353014865992f9151002cc4097fb986faf1
Reviewed-on: https://chromium-review.googlesource.com/1002775Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52506}
parent bbb26b5f
...@@ -5018,8 +5018,7 @@ void Heap::AddRetainedMap(Handle<Map> map) { ...@@ -5018,8 +5018,7 @@ void Heap::AddRetainedMap(Handle<Map> map) {
CompactRetainedMaps(*array); CompactRetainedMaps(*array);
} }
array = ArrayList::Add( 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()) { if (*array != retained_maps()) {
set_retained_maps(*array); set_retained_maps(*array);
} }
......
...@@ -10275,14 +10275,11 @@ Handle<FixedArrayOfWeakCells> FixedArrayOfWeakCells::Allocate( ...@@ -10275,14 +10275,11 @@ Handle<FixedArrayOfWeakCells> FixedArrayOfWeakCells::Allocate(
} }
// static // static
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(); int length = array->Length();
array = EnsureSpace(array, length + 1); array = EnsureSpace(array, length + 1);
if (mode == kReloadLengthAfterAllocation) { // Check that GC didn't remove elements from the array.
DCHECK(array->Length() <= length); DCHECK_EQ(array->Length(), length);
length = array->Length();
}
array->Set(length, *obj); array->Set(length, *obj);
array->SetLength(length + 1); array->SetLength(length + 1);
return array; return array;
...@@ -10290,12 +10287,11 @@ Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object> obj, ...@@ -10290,12 +10287,11 @@ Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object> obj,
// static // static
Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object> obj1, Handle<ArrayList> ArrayList::Add(Handle<ArrayList> array, Handle<Object> obj1,
Handle<Object> obj2, AddMode mode) { Handle<Object> obj2) {
int length = array->Length(); int length = array->Length();
array = EnsureSpace(array, length + 2); array = EnsureSpace(array, length + 2);
if (mode == kReloadLengthAfterAllocation) { // Check that GC didn't remove elements from the array.
length = array->Length(); DCHECK_EQ(array->Length(), length);
}
array->Set(length, *obj1); array->Set(length, *obj1);
array->Set(length + 1, *obj2); array->Set(length + 1, *obj2);
array->SetLength(length + 2); array->SetLength(length + 2);
......
...@@ -372,15 +372,9 @@ class FixedArrayOfWeakCells : public FixedArray { ...@@ -372,15 +372,9 @@ class FixedArrayOfWeakCells : public FixedArray {
// underlying FixedArray starting at kFirstIndex. // underlying FixedArray starting at kFirstIndex.
class ArrayList : public FixedArray { class ArrayList : public FixedArray {
public: public:
enum AddMode { static Handle<ArrayList> Add(Handle<ArrayList> array, Handle<Object> obj);
kNone,
// Use this if GC can delete elements from the 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, static Handle<ArrayList> Add(Handle<ArrayList> array, Handle<Object> obj1,
Handle<Object> obj2, AddMode = kNone); Handle<Object> obj2);
static Handle<ArrayList> New(Isolate* isolate, int size); static Handle<ArrayList> New(Isolate* isolate, int size);
// Returns the number of elements in the list, not the allocated size, which // Returns the number of elements in the list, not the allocated size, which
......
...@@ -4658,28 +4658,6 @@ TEST(MapRetaining) { ...@@ -4658,28 +4658,6 @@ TEST(MapRetaining) {
CheckMapRetainingFor(7); CheckMapRetainingFor(7);
} }
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();
AddRetainedMap(isolate, heap);
Handle<Map> map = Map::Create(isolate, 1);
CcTest::CollectGarbage(OLD_SPACE);
// Force GC in old space on next addition of retained map.
Map::WeakCellForMap(map);
heap::SimulateFullSpace(CcTest::heap()->new_space());
for (int i = 0; i < 10; i++) {
heap->AddRetainedMap(map);
}
CcTest::CollectGarbage(OLD_SPACE);
}
TEST(WritableVsImmortalRoots) { TEST(WritableVsImmortalRoots) {
for (int i = 0; i < Heap::kStrongRootListLength; ++i) { for (int i = 0; i < Heap::kStrongRootListLength; ++i) {
Heap::RootListIndex root_index = static_cast<Heap::RootListIndex>(i); Heap::RootListIndex root_index = static_cast<Heap::RootListIndex>(i);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment