Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: cherry-pick b93c80a from v8 upstream #7689

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions deps/v8/src/objects.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17389,7 +17389,7 @@ Handle<Derived> HashTable<Derived, Shape, Key>::EnsureCapacity(
int capacity = table->Capacity();
int nof = table->NumberOfElements() + n;

if (table->HasSufficientCapacity(n)) return table;
if (table->HasSufficientCapacityToAdd(n)) return table;

const int kMinCapacityForPretenure = 256;
bool should_pretenure = pretenure == TENURED ||
Expand All @@ -17405,16 +17405,16 @@ Handle<Derived> HashTable<Derived, Shape, Key>::EnsureCapacity(
return new_table;
}


template <typename Derived, typename Shape, typename Key>
bool HashTable<Derived, Shape, Key>::HasSufficientCapacity(int n) {
bool HashTable<Derived, Shape, Key>::HasSufficientCapacityToAdd(
int number_of_additional_elements) {
int capacity = Capacity();
int nof = NumberOfElements() + n;
int nof = NumberOfElements() + number_of_additional_elements;
int nod = NumberOfDeletedElements();
// Return true if:
// 50% is still free after adding n elements and
// 50% is still free after adding number_of_additional_elements elements and
// at most 50% of the free elements are deleted elements.
if (nod <= (capacity - nof) >> 1) {
if ((nof < capacity) && ((nod <= (capacity - nof) >> 1))) {
int needed_free = nof >> 1;
if (nof + needed_free <= capacity) return true;
}
Expand Down Expand Up @@ -18332,7 +18332,7 @@ void Dictionary<Derived, Shape, Key>::SetRequiresCopyOnCapacityChange() {
DCHECK_EQ(0, DerivedHashTable::NumberOfDeletedElements());
// Make sure that HashTable::EnsureCapacity will create a copy.
DerivedHashTable::SetNumberOfDeletedElements(DerivedHashTable::Capacity());
DCHECK(!DerivedHashTable::HasSufficientCapacity(1));
DCHECK(!DerivedHashTable::HasSufficientCapacityToAdd(1));
}


Expand Down Expand Up @@ -18740,6 +18740,19 @@ Handle<ObjectHashTable> ObjectHashTable::Put(Handle<ObjectHashTable> table,
if ((table->NumberOfDeletedElements() << 1) > table->NumberOfElements()) {
table->Rehash(isolate->factory()->undefined_value());
}
// If we're out of luck, we didn't get a GC recently, and so rehashing
// isn't enough to avoid a crash.
if (!table->HasSufficientCapacityToAdd(1)) {
int nof = table->NumberOfElements() + 1;
int capacity = ObjectHashTable::ComputeCapacity(nof * 2);
if (capacity > ObjectHashTable::kMaxCapacity) {
for (size_t i = 0; i < 2; ++i) {
isolate->heap()->CollectAllGarbage(
Heap::kFinalizeIncrementalMarkingMask, "full object hash table");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the calls to CollectAllGarbage() are necessary. Rehash() doesn't do heap allocations, does it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rehash can only free entries that are garbage, and we need at least two major GCs to guarantee that all garbage in weak sets/maps is found: the first GC might finish an already started GC round where a now garbage entry was still alive at the beginning of the GC round.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, it's a little implicit but your explanation makes sense.

}
table->Rehash(isolate->factory()->undefined_value());
}
}

// Check whether the hash table should be extended.
table = EnsureCapacity(table, 1, key);
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/objects.h
Original file line number Diff line number Diff line change
Expand Up @@ -3331,7 +3331,7 @@ class HashTable : public HashTableBase {
PretenureFlag pretenure = NOT_TENURED);

// Returns true if this table has sufficient capacity for adding n elements.
bool HasSufficientCapacity(int n);
bool HasSufficientCapacityToAdd(int number_of_additional_elements);

// Sets the capacity of the hash table.
void SetCapacity(int capacity) {
Expand Down