Skip to content

Commit

Permalink
deps: V8: cherry-pick cb1c2b0fbfe7
Browse files Browse the repository at this point in the history
Original commit message:

    Remove noscript_shared_function_infos

    SharedFunctionInfos that do not belong to a script were tracked in
    noscript_shared_function_infos. However this was only used in object-stats.
    Remove this since it was actually leaking memory in some use cases.

    Bug: v8:9674
    Change-Id: I9482f7e5dedf975666a70684b3d2ea04c9a23518
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1798423
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Michael Stanton <mvstanton@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63685}

Refs: v8/v8@cb1c2b0
  • Loading branch information
Flarna committed Apr 1, 2020
1 parent 4ee41c5 commit 52a2813
Show file tree
Hide file tree
Showing 12 changed files with 1 addition and 110 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.32',
'v8_embedder_string': '-node.33',

##### V8 defaults for Node.js #####

Expand Down
5 changes: 0 additions & 5 deletions deps/v8/src/heap/factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3501,11 +3501,6 @@ Handle<SharedFunctionInfo> Factory::NewSharedFunctionInfo(

share->clear_padding();
}
// Link into the list.
Handle<WeakArrayList> noscript_list = noscript_shared_function_infos();
noscript_list = WeakArrayList::AddToEnd(isolate(), noscript_list,
MaybeObjectHandle::Weak(share));
isolate()->heap()->set_noscript_shared_function_infos(*noscript_list);

#ifdef VERIFY_HEAP
share->SharedFunctionInfoVerify(isolate());
Expand Down
4 changes: 0 additions & 4 deletions deps/v8/src/heap/heap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ void Heap::SetRootStringTable(StringTable value) {
roots_table()[RootIndex::kStringTable] = value.ptr();
}

void Heap::SetRootNoScriptSharedFunctionInfos(Object value) {
roots_table()[RootIndex::kNoScriptSharedFunctionInfos] = value.ptr();
}

void Heap::SetMessageListeners(TemplateList value) {
roots_table()[RootIndex::kMessageListeners] = value.ptr();
}
Expand Down
7 changes: 0 additions & 7 deletions deps/v8/src/heap/heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5404,13 +5404,6 @@ void Heap::CompactWeakArrayLists(AllocationType allocation) {
DCHECK_IMPLIES(allocation == AllocationType::kOld, InOldSpace(*scripts));
scripts = CompactWeakArrayList(this, scripts, allocation);
set_script_list(*scripts);

Handle<WeakArrayList> no_script_list(noscript_shared_function_infos(),
isolate());
DCHECK_IMPLIES(allocation == AllocationType::kOld,
InOldSpace(*no_script_list));
no_script_list = CompactWeakArrayList(this, no_script_list, allocation);
set_noscript_shared_function_infos(*no_script_list);
}

void Heap::AddRetainedMap(Handle<Map> map) {
Expand Down
4 changes: 0 additions & 4 deletions deps/v8/src/heap/object-stats.cc
Original file line number Diff line number Diff line change
Expand Up @@ -829,10 +829,6 @@ void ObjectStatsCollectorImpl::CollectGlobalStatistics() {
ObjectStats::RETAINED_MAPS_TYPE);

// WeakArrayList.
RecordSimpleVirtualObjectStats(
HeapObject(),
WeakArrayList::cast(heap_->noscript_shared_function_infos()),
ObjectStats::NOSCRIPT_SHARED_FUNCTION_INFOS_TYPE);
RecordSimpleVirtualObjectStats(HeapObject(),
WeakArrayList::cast(heap_->script_list()),
ObjectStats::SCRIPT_LIST_TYPE);
Expand Down
1 change: 0 additions & 1 deletion deps/v8/src/heap/object-stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
V(MAP_PROTOTYPE_DICTIONARY_TYPE) \
V(MAP_PROTOTYPE_TYPE) \
V(MAP_STABLE_TYPE) \
V(NOSCRIPT_SHARED_FUNCTION_INFOS_TYPE) \
V(NUMBER_STRING_CACHE_TYPE) \
V(OBJECT_DICTIONARY_ELEMENTS_TYPE) \
V(OBJECT_ELEMENTS_TYPE) \
Expand Down
2 changes: 0 additions & 2 deletions deps/v8/src/heap/setup-heap-internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -907,8 +907,6 @@ void Heap::CreateInitialObjects() {
set_serialized_objects(roots.empty_fixed_array());
set_serialized_global_proxy_sizes(roots.empty_fixed_array());

set_noscript_shared_function_infos(roots.empty_weak_array_list());

/* Canonical off-heap trampoline data */
set_off_heap_trampoline_relocation_info(
*Builtins::GenerateOffHeapTrampolineRelocInfo(isolate_));
Expand Down
40 changes: 0 additions & 40 deletions deps/v8/src/objects/objects.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4988,24 +4988,6 @@ void SharedFunctionInfo::ScriptIterator::Reset(Isolate* isolate,
index_ = 0;
}

SharedFunctionInfo::GlobalIterator::GlobalIterator(Isolate* isolate)
: isolate_(isolate),
script_iterator_(isolate),
noscript_sfi_iterator_(isolate->heap()->noscript_shared_function_infos()),
sfi_iterator_(isolate, script_iterator_.Next()) {}

SharedFunctionInfo SharedFunctionInfo::GlobalIterator::Next() {
HeapObject next = noscript_sfi_iterator_.Next();
if (!next.is_null()) return SharedFunctionInfo::cast(next);
for (;;) {
next = sfi_iterator_.Next();
if (!next.is_null()) return SharedFunctionInfo::cast(next);
Script next_script = script_iterator_.Next();
if (next_script.is_null()) return SharedFunctionInfo();
sfi_iterator_.Reset(isolate_, next_script);
}
}

void SharedFunctionInfo::SetScript(Handle<SharedFunctionInfo> shared,
Handle<Object> script_object,
int function_literal_id,
Expand Down Expand Up @@ -5036,30 +5018,8 @@ void SharedFunctionInfo::SetScript(Handle<SharedFunctionInfo> shared,
}
#endif
list->Set(function_literal_id, HeapObjectReference::Weak(*shared));

// Remove shared function info from root array.
WeakArrayList noscript_list =
isolate->heap()->noscript_shared_function_infos();
CHECK(noscript_list.RemoveOne(MaybeObjectHandle::Weak(shared)));
} else {
DCHECK(shared->script().IsScript());
Handle<WeakArrayList> list =
isolate->factory()->noscript_shared_function_infos();

#ifdef DEBUG
if (FLAG_enable_slow_asserts) {
WeakArrayList::Iterator iterator(*list);
for (HeapObject next = iterator.Next(); !next.is_null();
next = iterator.Next()) {
DCHECK_NE(next, *shared);
}
}
#endif // DEBUG

list =
WeakArrayList::AddToEnd(isolate, list, MaybeObjectHandle::Weak(shared));

isolate->heap()->SetRootNoScriptSharedFunctionInfos(*list);

// Remove shared function info from old script's list.
Script old_script = Script::cast(shared->script());
Expand Down
15 changes: 0 additions & 15 deletions deps/v8/src/objects/shared-function-info.h
Original file line number Diff line number Diff line change
Expand Up @@ -640,21 +640,6 @@ class SharedFunctionInfo : public HeapObject {
DISALLOW_COPY_AND_ASSIGN(ScriptIterator);
};

// Iterate over all shared function infos on the heap.
class GlobalIterator {
public:
V8_EXPORT_PRIVATE explicit GlobalIterator(Isolate* isolate);
V8_EXPORT_PRIVATE SharedFunctionInfo Next();

private:
Isolate* isolate_;
Script::Iterator script_iterator_;
WeakArrayList::Iterator noscript_sfi_iterator_;
SharedFunctionInfo::ScriptIterator sfi_iterator_;
DISALLOW_HEAP_ALLOCATION(no_gc_)
DISALLOW_COPY_AND_ASSIGN(GlobalIterator);
};

DECL_CAST(SharedFunctionInfo)

// Constants.
Expand Down
2 changes: 0 additions & 2 deletions deps/v8/src/roots/roots.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,6 @@ class Symbol;
/* Feedback vectors that we need for code coverage or type profile */ \
V(Object, feedback_vectors_for_profiling_tools, \
FeedbackVectorsForProfilingTools) \
V(WeakArrayList, noscript_shared_function_infos, \
NoScriptSharedFunctionInfos) \
V(FixedArray, serialized_objects, SerializedObjects) \
V(FixedArray, serialized_global_proxy_sizes, SerializedGlobalProxySizes) \
V(TemplateList, message_listeners, MessageListeners) \
Expand Down
28 changes: 0 additions & 28 deletions deps/v8/test/cctest/heap/test-heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5394,34 +5394,6 @@ TEST(ScriptIterator) {
CHECK_EQ(0, script_count);
}


TEST(SharedFunctionInfoIterator) {
CcTest::InitializeVM();
v8::HandleScope scope(CcTest::isolate());
Isolate* isolate = CcTest::i_isolate();
Heap* heap = CcTest::heap();
LocalContext context;

CcTest::CollectAllGarbage();
CcTest::CollectAllGarbage();

int sfi_count = 0;
{
HeapObjectIterator it(heap);
for (HeapObject obj = it.Next(); !obj.is_null(); obj = it.Next()) {
if (!obj.IsSharedFunctionInfo()) continue;
sfi_count++;
}
}

{
SharedFunctionInfo::GlobalIterator iterator(isolate);
while (!iterator.Next().is_null()) sfi_count--;
}

CHECK_EQ(0, sfi_count);
}

// This is the same as Factory::NewByteArray, except it doesn't retry on
// allocation failure.
AllocationResult HeapTester::AllocateByteArrayForTest(
Expand Down
1 change: 0 additions & 1 deletion deps/v8/test/cctest/test-roots.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ bool IsInitiallyMutable(Factory* factory, Address object_address) {
V(dirty_js_finalization_groups) \
V(feedback_vectors_for_profiling_tools) \
V(materialized_objects) \
V(noscript_shared_function_infos) \
V(public_symbol_table) \
V(retained_maps) \
V(retaining_path_targets) \
Expand Down

0 comments on commit 52a2813

Please sign in to comment.