Commit 3745c625 authored by Marja Hölttä's avatar Marja Hölttä Committed by Commit Bot

[in-place weak refs] Make WeakArrayList::RemoveOne more efficient

Previously, removing an element in the middle made it consume space
forever. This fixes that, without changing the complexity of removal /
addition. The trade-off is that RemoveOne will shuffle indices (which should be
OK for the current users).

BUG=v8:7308

Change-Id: I0373e30f2d9d1ffb93a78d383d41b500dbbf3429
Reviewed-on: https://chromium-review.googlesource.com/1159371
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54875}
parent 832c69d3
......@@ -10612,13 +10612,14 @@ int WeakArrayList::CountLiveWeakReferences() const {
bool WeakArrayList::RemoveOne(MaybeObjectHandle value) {
if (length() == 0) return false;
// Optimize for the most recently added element to be removed again.
for (int i = length() - 1; i >= 0; --i) {
int last_index = length() - 1;
for (int i = last_index; i >= 0; --i) {
if (Get(i) == *value) {
// Users should make sure that there are no duplicates.
Set(i, HeapObjectReference::ClearedValue());
if (i == length() - 1) {
set_length(length() - 1);
}
// Move the last element into the this slot (or no-op, if this is the
// last slot).
Set(i, Get(last_index));
Set(last_index, HeapObjectReference::ClearedValue());
set_length(last_index);
return true;
}
}
......
......@@ -370,7 +370,10 @@ class WeakArrayList : public HeapObject {
// Returns the number of non-cleaned weak references in the array.
int CountLiveWeakReferences() const;
// Returns whether an entry was found and removed.
// Returns whether an entry was found and removed. Will move the elements
// around in the array - this method can only be used in cases where the user
// doesn't care about the indices! Users should make sure there are no
// duplicates.
bool RemoveOne(MaybeObjectHandle value);
class Iterator {
......
......@@ -564,31 +564,24 @@ TEST(WeakArrayListRemove) {
CHECK(array->RemoveOne(MaybeObjectHandle::Weak(elem1)));
CHECK_EQ(array->length(), 3);
CHECK_EQ(array->length(), 2);
CHECK_EQ(array->Get(0), HeapObjectReference::Weak(*elem0));
CHECK(array->Get(1)->IsClearedWeakHeapObject());
CHECK_EQ(array->Get(2), HeapObjectReference::Weak(*elem2));
CHECK_EQ(array->Get(1), HeapObjectReference::Weak(*elem2));
CHECK(!array->RemoveOne(MaybeObjectHandle::Weak(elem1)));
CHECK_EQ(array->length(), 3);
CHECK_EQ(array->length(), 2);
CHECK_EQ(array->Get(0), HeapObjectReference::Weak(*elem0));
CHECK(array->Get(1)->IsClearedWeakHeapObject());
CHECK_EQ(array->Get(2), HeapObjectReference::Weak(*elem2));
CHECK_EQ(array->Get(1), HeapObjectReference::Weak(*elem2));
CHECK(array->RemoveOne(MaybeObjectHandle::Weak(elem0)));
CHECK_EQ(array->length(), 3);
CHECK(array->Get(0)->IsClearedWeakHeapObject());
CHECK(array->Get(1)->IsClearedWeakHeapObject());
CHECK_EQ(array->Get(2), HeapObjectReference::Weak(*elem2));
CHECK_EQ(array->length(), 1);
CHECK_EQ(array->Get(0), HeapObjectReference::Weak(*elem2));
CHECK(array->RemoveOne(MaybeObjectHandle::Weak(elem2)));
// Removing the last element decreases the array length.
CHECK_EQ(array->length(), 2);
CHECK(array->Get(0)->IsClearedWeakHeapObject());
CHECK(array->Get(1)->IsClearedWeakHeapObject());
CHECK_EQ(array->length(), 0);
}
TEST(Regress7768) {
......
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