From a83fc49717db3943e4b71d02ef444564abf8d98c Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Mon, 24 Feb 2020 22:44:44 +0100 Subject: [PATCH] deps: V8: cherry-pick cb1c2b0fbfe7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Michael Stanton Commit-Queue: Dominik Inführ Cr-Commit-Position: refs/heads/master@{#63685} PR-URL: https://github.com/nodejs/node/pull/31939 Refs: https://github.com/v8/v8/commit/cb1c2b0fbfe730ed46d2b7a1ad3f58a21facc477 Refs: https://github.com/nodejs/node/issues/31914 Reviewed-By: Ben Noordhuis Reviewed-By: Michaël Zasso Reviewed-By: Matheus Marchini --- common.gypi | 2 +- deps/v8/src/heap/factory.cc | 5 --- deps/v8/src/heap/heap-inl.h | 4 --- deps/v8/src/heap/heap.cc | 7 ---- deps/v8/src/heap/object-stats.cc | 4 --- deps/v8/src/heap/object-stats.h | 1 - deps/v8/src/heap/setup-heap-internal.cc | 2 -- deps/v8/src/objects/objects.cc | 40 ---------------------- deps/v8/src/objects/shared-function-info.h | 15 -------- deps/v8/src/roots/roots.h | 2 -- deps/v8/test/cctest/heap/test-heap.cc | 28 --------------- deps/v8/test/cctest/test-roots.cc | 1 - 12 files changed, 1 insertion(+), 110 deletions(-) diff --git a/common.gypi b/common.gypi index e497e103627fe5..a3216401be7eb9 100644 --- a/common.gypi +++ b/common.gypi @@ -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 ##### diff --git a/deps/v8/src/heap/factory.cc b/deps/v8/src/heap/factory.cc index dc3404969bc093..92728341054758 100644 --- a/deps/v8/src/heap/factory.cc +++ b/deps/v8/src/heap/factory.cc @@ -3501,11 +3501,6 @@ Handle Factory::NewSharedFunctionInfo( share->clear_padding(); } - // Link into the list. - Handle 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()); diff --git a/deps/v8/src/heap/heap-inl.h b/deps/v8/src/heap/heap-inl.h index da803f33395364..01d208b1bd63ed 100644 --- a/deps/v8/src/heap/heap-inl.h +++ b/deps/v8/src/heap/heap-inl.h @@ -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(); } diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index ff3b34cfb4f29b..3caeda8bdd161b 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -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 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) { diff --git a/deps/v8/src/heap/object-stats.cc b/deps/v8/src/heap/object-stats.cc index 2ee88361c965f3..d79e1c6d0fea4d 100644 --- a/deps/v8/src/heap/object-stats.cc +++ b/deps/v8/src/heap/object-stats.cc @@ -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); diff --git a/deps/v8/src/heap/object-stats.h b/deps/v8/src/heap/object-stats.h index 2a9b9675ef2145..28ef967c5ca011 100644 --- a/deps/v8/src/heap/object-stats.h +++ b/deps/v8/src/heap/object-stats.h @@ -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) \ diff --git a/deps/v8/src/heap/setup-heap-internal.cc b/deps/v8/src/heap/setup-heap-internal.cc index 15ca6d7930383a..46f073a3cd87b3 100644 --- a/deps/v8/src/heap/setup-heap-internal.cc +++ b/deps/v8/src/heap/setup-heap-internal.cc @@ -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_)); diff --git a/deps/v8/src/objects/objects.cc b/deps/v8/src/objects/objects.cc index 11755666650a05..5e03430a32348a 100644 --- a/deps/v8/src/objects/objects.cc +++ b/deps/v8/src/objects/objects.cc @@ -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 shared, Handle script_object, int function_literal_id, @@ -5036,30 +5018,8 @@ void SharedFunctionInfo::SetScript(Handle 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 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()); diff --git a/deps/v8/src/objects/shared-function-info.h b/deps/v8/src/objects/shared-function-info.h index dc84653ede2987..997bb33863388b 100644 --- a/deps/v8/src/objects/shared-function-info.h +++ b/deps/v8/src/objects/shared-function-info.h @@ -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. diff --git a/deps/v8/src/roots/roots.h b/deps/v8/src/roots/roots.h index c82ec6d04f8e33..4fa6dfdfbf8f04 100644 --- a/deps/v8/src/roots/roots.h +++ b/deps/v8/src/roots/roots.h @@ -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) \ diff --git a/deps/v8/test/cctest/heap/test-heap.cc b/deps/v8/test/cctest/heap/test-heap.cc index 48ba63a5dbf992..10bd50294ef6f0 100644 --- a/deps/v8/test/cctest/heap/test-heap.cc +++ b/deps/v8/test/cctest/heap/test-heap.cc @@ -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( diff --git a/deps/v8/test/cctest/test-roots.cc b/deps/v8/test/cctest/test-roots.cc index d04190363978a4..5a81f8e9ce96b1 100644 --- a/deps/v8/test/cctest/test-roots.cc +++ b/deps/v8/test/cctest/test-roots.cc @@ -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) \