Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 97a3f7b

Browse filesBrowse files
mmarchiniBethGriggs
mmarchini
authored and
BethGriggs
committed
deps: V8: backport fb26d0bb1835
Original commit message: [objects] Compact and shrink script_list So far creating scripts always grew the script_list without ever reusing cleared slots or shrinking. While this is probably not a problem with script_list in practice, this is still a memory leak. Fix this leak by using WeakArrayList::Append instead of AddToEnd. Append adds to the end of the array, but potentially compacts and shrinks the list as well. Other WeakArrayLists can use this method as well, as long as they are not using indices into this array. Bug: v8:10031 Change-Id: If743c4cc3f8d67ab735522f0ded038b2fb43e437 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967385 Commit-Queue: Dominik Inführ <dinfuehr@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#65640} Refs: v8/v8@fb26d0b PR-URL: #33573 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Shelley Vohr <codebytere@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 136d8ea commit 97a3f7b
Copy full SHA for 97a3f7b

File tree

Expand file treeCollapse file tree

7 files changed

+215
-16
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+215
-16
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
# Reset this number to 0 on major V8 upgrades.
3636
# Increment by one for each non-official patch applied to deps/v8.
37-
'v8_embedder_string': '-node.38',
37+
'v8_embedder_string': '-node.39',
3838

3939
##### V8 defaults for Node.js #####
4040

Collapse file

‎deps/v8/src/heap/factory.cc‎

Copy file name to clipboardExpand all lines: deps/v8/src/heap/factory.cc
+55-12Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,8 +1677,8 @@ Handle<Script> Factory::NewScriptWithId(Handle<String> source, int script_id,
16771677
script->set_flags(0);
16781678
script->set_host_defined_options(*empty_fixed_array());
16791679
Handle<WeakArrayList> scripts = script_list();
1680-
scripts = WeakArrayList::AddToEnd(isolate(), scripts,
1681-
MaybeObjectHandle::Weak(script));
1680+
scripts = WeakArrayList::Append(isolate(), scripts,
1681+
MaybeObjectHandle::Weak(script));
16821682
heap->set_script_list(*scripts);
16831683
LOG(isolate(), ScriptEvent(Logger::ScriptEventType::kCreate, script_id));
16841684
TRACE_EVENT_OBJECT_CREATED_WITH_ID(
@@ -2112,6 +2112,29 @@ Handle<FixedArray> Factory::CopyFixedArrayAndGrow(Handle<FixedArray> array,
21122112
return CopyArrayAndGrow(array, grow_by, allocation);
21132113
}
21142114

2115+
Handle<WeakArrayList> Factory::NewUninitializedWeakArrayList(
2116+
int capacity, AllocationType allocation) {
2117+
DCHECK_LE(0, capacity);
2118+
if (capacity == 0) return empty_weak_array_list();
2119+
2120+
HeapObject obj = AllocateRawWeakArrayList(capacity, allocation);
2121+
obj.set_map_after_allocation(*weak_array_list_map(), SKIP_WRITE_BARRIER);
2122+
2123+
Handle<WeakArrayList> result(WeakArrayList::cast(obj), isolate());
2124+
result->set_length(0);
2125+
result->set_capacity(capacity);
2126+
return result;
2127+
}
2128+
2129+
Handle<WeakArrayList> Factory::NewWeakArrayList(int capacity,
2130+
AllocationType allocation) {
2131+
Handle<WeakArrayList> result =
2132+
NewUninitializedWeakArrayList(capacity, allocation);
2133+
MemsetTagged(ObjectSlot(result->data_start()),
2134+
ReadOnlyRoots(isolate()).undefined_value(), capacity);
2135+
return result;
2136+
}
2137+
21152138
Handle<WeakFixedArray> Factory::CopyWeakFixedArrayAndGrow(
21162139
Handle<WeakFixedArray> src, int grow_by, AllocationType allocation) {
21172140
DCHECK(!src->IsTransitionArray()); // Compacted by GC, this code doesn't work
@@ -2123,22 +2146,42 @@ Handle<WeakArrayList> Factory::CopyWeakArrayListAndGrow(
21232146
int old_capacity = src->capacity();
21242147
int new_capacity = old_capacity + grow_by;
21252148
DCHECK_GE(new_capacity, old_capacity);
2126-
HeapObject obj = AllocateRawWeakArrayList(new_capacity, allocation);
2127-
obj.set_map_after_allocation(src->map(), SKIP_WRITE_BARRIER);
2128-
2129-
WeakArrayList result = WeakArrayList::cast(obj);
2149+
Handle<WeakArrayList> result =
2150+
NewUninitializedWeakArrayList(new_capacity, allocation);
21302151
int old_len = src->length();
2131-
result.set_length(old_len);
2132-
result.set_capacity(new_capacity);
2152+
result->set_length(old_len);
21332153

21342154
// Copy the content.
21352155
DisallowHeapAllocation no_gc;
2136-
WriteBarrierMode mode = obj.GetWriteBarrierMode(no_gc);
2137-
result.CopyElements(isolate(), 0, *src, 0, old_len, mode);
2138-
MemsetTagged(ObjectSlot(result.data_start() + old_len),
2156+
WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
2157+
result->CopyElements(isolate(), 0, *src, 0, old_len, mode);
2158+
MemsetTagged(ObjectSlot(result->data_start() + old_len),
21392159
ReadOnlyRoots(isolate()).undefined_value(),
21402160
new_capacity - old_len);
2141-
return Handle<WeakArrayList>(result, isolate());
2161+
return result;
2162+
}
2163+
2164+
Handle<WeakArrayList> Factory::CompactWeakArrayList(Handle<WeakArrayList> src,
2165+
int new_capacity,
2166+
AllocationType allocation) {
2167+
Handle<WeakArrayList> result =
2168+
NewUninitializedWeakArrayList(new_capacity, allocation);
2169+
2170+
// Copy the content.
2171+
DisallowHeapAllocation no_gc;
2172+
WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
2173+
int copy_to = 0, length = src->length();
2174+
for (int i = 0; i < length; i++) {
2175+
MaybeObject element = src->Get(i);
2176+
if (element->IsCleared()) continue;
2177+
result->Set(copy_to++, element, mode);
2178+
}
2179+
result->set_length(copy_to);
2180+
2181+
MemsetTagged(ObjectSlot(result->data_start() + copy_to),
2182+
ReadOnlyRoots(isolate()).undefined_value(),
2183+
new_capacity - copy_to);
2184+
return result;
21422185
}
21432186

21442187
Handle<PropertyArray> Factory::CopyPropertyArrayAndGrow(
Collapse file

‎deps/v8/src/heap/factory.h‎

Copy file name to clipboardExpand all lines: deps/v8/src/heap/factory.h
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,9 @@ class V8_EXPORT_PRIVATE Factory {
524524

525525
Handle<JSObject> NewFunctionPrototype(Handle<JSFunction> function);
526526

527+
Handle<WeakArrayList> NewWeakArrayList(
528+
int capacity, AllocationType allocation = AllocationType::kYoung);
529+
527530
Handle<WeakCell> NewWeakCell();
528531

529532
// Returns a deep copy of the JavaScript object.
@@ -549,6 +552,10 @@ class V8_EXPORT_PRIVATE Factory {
549552
Handle<WeakArrayList> array, int grow_by,
550553
AllocationType allocation = AllocationType::kYoung);
551554

555+
Handle<WeakArrayList> CompactWeakArrayList(
556+
Handle<WeakArrayList> array, int new_capacity,
557+
AllocationType allocation = AllocationType::kYoung);
558+
552559
Handle<PropertyArray> CopyPropertyArrayAndGrow(
553560
Handle<PropertyArray> array, int grow_by,
554561
AllocationType allocation = AllocationType::kYoung);
@@ -1111,6 +1118,10 @@ class V8_EXPORT_PRIVATE Factory {
11111118
// Initializes JSObject body starting at given offset.
11121119
void InitializeJSObjectBody(Handle<JSObject> obj, Handle<Map> map,
11131120
int start_offset);
1121+
1122+
private:
1123+
Handle<WeakArrayList> NewUninitializedWeakArrayList(
1124+
int capacity, AllocationType allocation = AllocationType::kYoung);
11141125
};
11151126

11161127
// Utility class to simplify argument handling around JSFunction creation.
Collapse file

‎deps/v8/src/objects/fixed-array.h‎

Copy file name to clipboardExpand all lines: deps/v8/src/objects/fixed-array.h
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,17 @@ class WeakArrayList : public HeapObject {
339339
Isolate* isolate, Handle<WeakArrayList> array,
340340
const MaybeObjectHandle& value);
341341

342+
// Appends an element to the array and possibly compacts and shrinks live weak
343+
// references to the start of the collection. Only use this method when
344+
// indices to elements can change.
345+
static Handle<WeakArrayList> Append(
346+
Isolate* isolate, Handle<WeakArrayList> array,
347+
const MaybeObjectHandle& value,
348+
AllocationType allocation = AllocationType::kYoung);
349+
350+
// Compact weak references to the beginning of the array.
351+
V8_EXPORT_PRIVATE void Compact(Isolate* isolate);
352+
342353
inline MaybeObject Get(int index) const;
343354
inline MaybeObject Get(Isolate* isolate, int index) const;
344355

@@ -352,6 +363,10 @@ class WeakArrayList : public HeapObject {
352363
return kHeaderSize + capacity * kTaggedSize;
353364
}
354365

366+
static constexpr int CapacityForLength(int length) {
367+
return length + Max(length / 2, 2);
368+
}
369+
355370
// Gives access to raw memory which stores the array's data.
356371
inline MaybeObjectSlot data_start();
357372

@@ -383,6 +398,9 @@ class WeakArrayList : public HeapObject {
383398
// Returns the number of non-cleaned weak references in the array.
384399
int CountLiveWeakReferences() const;
385400

401+
// Returns the number of non-cleaned elements in the array.
402+
int CountLiveElements() const;
403+
386404
// Returns whether an entry was found and removed. Will move the elements
387405
// around in the array - this method can only be used in cases where the user
388406
// doesn't care about the indices! Users should make sure there are no
Collapse file

‎deps/v8/src/objects/objects.cc‎

Copy file name to clipboardExpand all lines: deps/v8/src/objects/objects.cc
+70-3Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3952,6 +3952,65 @@ Handle<WeakArrayList> WeakArrayList::AddToEnd(Isolate* isolate,
39523952
return array;
39533953
}
39543954

3955+
// static
3956+
Handle<WeakArrayList> WeakArrayList::Append(Isolate* isolate,
3957+
Handle<WeakArrayList> array,
3958+
const MaybeObjectHandle& value,
3959+
AllocationType allocation) {
3960+
int length = array->length();
3961+
3962+
if (length < array->capacity()) {
3963+
array->Set(length, *value);
3964+
array->set_length(length + 1);
3965+
return array;
3966+
}
3967+
3968+
// Not enough space in the array left, either grow, shrink or
3969+
// compact the array.
3970+
int new_length = array->CountLiveElements() + 1;
3971+
3972+
bool shrink = new_length < length / 4;
3973+
bool grow = 3 * (length / 4) < new_length;
3974+
3975+
if (shrink || grow) {
3976+
// Grow or shrink array and compact out-of-place.
3977+
int new_capacity = CapacityForLength(new_length);
3978+
array = isolate->factory()->CompactWeakArrayList(array, new_capacity,
3979+
allocation);
3980+
3981+
} else {
3982+
// Perform compaction in the current array.
3983+
array->Compact(isolate);
3984+
}
3985+
3986+
// Now append value to the array, there should always be enough space now.
3987+
DCHECK_LT(array->length(), array->capacity());
3988+
3989+
// Reload length, allocation might have killed some weak refs.
3990+
int index = array->length();
3991+
array->Set(index, *value);
3992+
array->set_length(index + 1);
3993+
return array;
3994+
}
3995+
3996+
void WeakArrayList::Compact(Isolate* isolate) {
3997+
int length = this->length();
3998+
int new_length = 0;
3999+
4000+
for (int i = 0; i < length; i++) {
4001+
MaybeObject value = Get(isolate, i);
4002+
4003+
if (!value->IsCleared()) {
4004+
if (new_length != i) {
4005+
Set(new_length, value);
4006+
}
4007+
++new_length;
4008+
}
4009+
}
4010+
4011+
set_length(new_length);
4012+
}
4013+
39554014
bool WeakArrayList::IsFull() { return length() == capacity(); }
39564015

39574016
// static
@@ -3961,9 +4020,7 @@ Handle<WeakArrayList> WeakArrayList::EnsureSpace(Isolate* isolate,
39614020
AllocationType allocation) {
39624021
int capacity = array->capacity();
39634022
if (capacity < length) {
3964-
int new_capacity = length;
3965-
new_capacity = new_capacity + Max(new_capacity / 2, 2);
3966-
int grow_by = new_capacity - capacity;
4023+
int grow_by = CapacityForLength(length) - capacity;
39674024
array = isolate->factory()->CopyWeakArrayListAndGrow(array, grow_by,
39684025
allocation);
39694026
}
@@ -3980,6 +4037,16 @@ int WeakArrayList::CountLiveWeakReferences() const {
39804037
return live_weak_references;
39814038
}
39824039

4040+
int WeakArrayList::CountLiveElements() const {
4041+
int non_cleared_objects = 0;
4042+
for (int i = 0; i < length(); i++) {
4043+
if (!Get(i)->IsCleared()) {
4044+
++non_cleared_objects;
4045+
}
4046+
}
4047+
return non_cleared_objects;
4048+
}
4049+
39834050
bool WeakArrayList::RemoveOne(const MaybeObjectHandle& value) {
39844051
if (length() == 0) return false;
39854052
// Optimize for the most recently added element to be removed again.
Collapse file

‎deps/v8/test/unittests/BUILD.gn‎

Copy file name to clipboardExpand all lines: deps/v8/test/unittests/BUILD.gn
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ v8_source_set("unittests_sources") {
190190
"numbers/conversions-unittest.cc",
191191
"objects/object-unittest.cc",
192192
"objects/value-serializer-unittest.cc",
193+
"objects/weakarraylist-unittest.cc",
193194
"parser/ast-value-unittest.cc",
194195
"parser/preparser-unittest.cc",
195196
"profiler/strings-storage-unittest.cc",
Collapse file

‎deps/v8/test/unittests/objects/weakarraylist-unittest.cc‎

Copy file name to clipboard
+59Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright 2019 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "src/heap/factory.h"
6+
#include "test/unittests/test-utils.h"
7+
8+
namespace v8 {
9+
namespace internal {
10+
11+
using WeakArrayListTest = TestWithIsolate;
12+
13+
TEST_F(WeakArrayListTest, Compact) {
14+
Handle<WeakArrayList> list = isolate()->factory()->NewWeakArrayList(10);
15+
EXPECT_EQ(list->length(), 0);
16+
EXPECT_EQ(list->capacity(), 10);
17+
18+
MaybeObject some_object =
19+
MaybeObject::FromObject(*isolate()->factory()->empty_fixed_array());
20+
MaybeObject weak_ref = MaybeObject::MakeWeak(some_object);
21+
MaybeObject smi = MaybeObject::FromSmi(Smi::FromInt(0));
22+
MaybeObject cleared_ref = HeapObjectReference::ClearedValue(isolate());
23+
list->Set(0, weak_ref);
24+
list->Set(1, smi);
25+
list->Set(2, cleared_ref);
26+
list->Set(3, cleared_ref);
27+
list->set_length(5);
28+
29+
list->Compact(isolate());
30+
EXPECT_EQ(list->length(), 3);
31+
EXPECT_EQ(list->capacity(), 10);
32+
}
33+
34+
TEST_F(WeakArrayListTest, OutOfPlaceCompact) {
35+
Handle<WeakArrayList> list = isolate()->factory()->NewWeakArrayList(20);
36+
EXPECT_EQ(list->length(), 0);
37+
EXPECT_EQ(list->capacity(), 20);
38+
39+
MaybeObject some_object =
40+
MaybeObject::FromObject(*isolate()->factory()->empty_fixed_array());
41+
MaybeObject weak_ref = MaybeObject::MakeWeak(some_object);
42+
MaybeObject smi = MaybeObject::FromSmi(Smi::FromInt(0));
43+
MaybeObject cleared_ref = HeapObjectReference::ClearedValue(isolate());
44+
list->Set(0, weak_ref);
45+
list->Set(1, smi);
46+
list->Set(2, cleared_ref);
47+
list->Set(3, smi);
48+
list->Set(4, cleared_ref);
49+
list->set_length(6);
50+
51+
Handle<WeakArrayList> compacted =
52+
isolate()->factory()->CompactWeakArrayList(list, 4);
53+
EXPECT_EQ(list->length(), 6);
54+
EXPECT_EQ(compacted->length(), 4);
55+
EXPECT_EQ(compacted->capacity(), 4);
56+
}
57+
58+
} // namespace internal
59+
} // namespace v8

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.