From 3f3251e429908f5b2f96a22b67702a3b6cde8f59 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 10 Oct 2022 08:33:18 +0000 Subject: [PATCH 1/2] deps: V8: cherry-pick b95354290941 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [extensions] Fix dcheck failures in getV8Statistics HeapObjectIterator creates a SafepointScope which requires the heap to allow garbage collection. This collides with the outer DisallowGarbageCollection scope. HeapObjectIterator already ensures there is no allocation during its lifetime, so there is no need to create an outer DisallowGarbageCollection scope. Code::source_position_table requires their kind not equals to CodeKind::BASELINE. This also exposes the statistics extension through flag --expose-statistics. Bug: v8:12657 Change-Id: I1bf11cf499285a742dd99ec8c228ebc36152b597 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3496552 Reviewed-by: Camillo Bruni Reviewed-by: Marja Hölttä Commit-Queue: Chengzhong Wu Cr-Commit-Position: refs/heads/main@{#79425} Refs: https://github.com/v8/v8/commit/b953542909416a5be11220d9adf6da1aff1f009c --- common.gypi | 2 +- .../v8/src/extensions/statistics-extension.cc | 45 ++++++++++--------- deps/v8/src/flags/flag-definitions.h | 1 + deps/v8/src/init/bootstrapper.cc | 2 +- deps/v8/test/mjsunit/statistics-extension.js | 12 +++++ 5 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 deps/v8/test/mjsunit/statistics-extension.js diff --git a/common.gypi b/common.gypi index d68a4c9eafacfa..4e9c78827e6414 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,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.22', + 'v8_embedder_string': '-node.23', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/extensions/statistics-extension.cc b/deps/v8/src/extensions/statistics-extension.cc index 1911dfc39e82a4..3a1f2785875821 100644 --- a/deps/v8/src/extensions/statistics-extension.cc +++ b/deps/v8/src/extensions/statistics-extension.cc @@ -124,36 +124,41 @@ void StatisticsExtension::GetCounters( AddNumber64(args.GetIsolate(), result, heap->external_memory(), "amount_of_external_allocated_memory"); - args.GetReturnValue().Set(result); - DisallowGarbageCollection no_gc; - HeapObjectIterator iterator( - reinterpret_cast(args.GetIsolate())->heap()); int reloc_info_total = 0; int source_position_table_total = 0; - for (HeapObject obj = iterator.Next(); !obj.is_null(); - obj = iterator.Next()) { - Object maybe_source_positions; - if (obj.IsCode()) { - Code code = Code::cast(obj); - reloc_info_total += code.relocation_info().Size(); - maybe_source_positions = code.source_position_table(); - } else if (obj.IsBytecodeArray()) { - maybe_source_positions = - BytecodeArray::cast(obj).source_position_table(kAcquireLoad); - } else { - continue; + { + HeapObjectIterator iterator( + reinterpret_cast(args.GetIsolate())->heap()); + DCHECK(!AllowGarbageCollection::IsAllowed()); + for (HeapObject obj = iterator.Next(); !obj.is_null(); + obj = iterator.Next()) { + Object maybe_source_positions; + if (obj.IsCode()) { + Code code = Code::cast(obj); + reloc_info_total += code.relocation_info().Size(); + // Baseline code doesn't have source positions since it uses + // interpreter code positions. + if (code.kind() == CodeKind::BASELINE) continue; + maybe_source_positions = code.source_position_table(); + } else if (obj.IsBytecodeArray()) { + maybe_source_positions = + BytecodeArray::cast(obj).source_position_table(kAcquireLoad); + } else { + continue; + } + if (!maybe_source_positions.IsByteArray()) continue; + ByteArray source_positions = ByteArray::cast(maybe_source_positions); + if (source_positions.length() == 0) continue; + source_position_table_total += source_positions.Size(); } - if (!maybe_source_positions.IsByteArray()) continue; - ByteArray source_positions = ByteArray::cast(maybe_source_positions); - if (source_positions.length() == 0) continue; - source_position_table_total += source_positions.Size(); } AddNumber(args.GetIsolate(), result, reloc_info_total, "reloc_info_total_size"); AddNumber(args.GetIsolate(), result, source_position_table_total, "source_position_table_total_size"); + args.GetReturnValue().Set(result); } } // namespace internal diff --git a/deps/v8/src/flags/flag-definitions.h b/deps/v8/src/flags/flag-definitions.h index d8e1dd6b9c133a..2b865da812be50 100644 --- a/deps/v8/src/flags/flag-definitions.h +++ b/deps/v8/src/flags/flag-definitions.h @@ -1390,6 +1390,7 @@ DEFINE_STRING(expose_gc_as, nullptr, DEFINE_IMPLICATION(expose_gc_as, expose_gc) DEFINE_BOOL(expose_externalize_string, false, "expose externalize string extension") +DEFINE_BOOL(expose_statistics, false, "expose statistics extension") DEFINE_BOOL(expose_trigger_failure, false, "expose trigger-failure extension") DEFINE_BOOL(expose_ignition_statistics, false, "expose ignition-statistics extension (requires building with " diff --git a/deps/v8/src/init/bootstrapper.cc b/deps/v8/src/init/bootstrapper.cc index 326944e13e9020..7b868f9e9bacca 100644 --- a/deps/v8/src/init/bootstrapper.cc +++ b/deps/v8/src/init/bootstrapper.cc @@ -5089,7 +5089,7 @@ bool Genesis::InstallExtensions(Isolate* isolate, InstallExtension(isolate, "v8/gc", &extension_states)) && (!FLAG_expose_externalize_string || InstallExtension(isolate, "v8/externalize", &extension_states)) && - (!TracingFlags::is_gc_stats_enabled() || + (!(FLAG_expose_statistics || TracingFlags::is_gc_stats_enabled()) || InstallExtension(isolate, "v8/statistics", &extension_states)) && (!FLAG_expose_trigger_failure || InstallExtension(isolate, "v8/trigger-failure", &extension_states)) && diff --git a/deps/v8/test/mjsunit/statistics-extension.js b/deps/v8/test/mjsunit/statistics-extension.js new file mode 100644 index 00000000000000..b6573736bbab2e --- /dev/null +++ b/deps/v8/test/mjsunit/statistics-extension.js @@ -0,0 +1,12 @@ +// Copyright 2022 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --expose-statistics + +assertEquals(typeof getV8Statistics, 'function'); +var result = getV8Statistics(); +assertEquals(typeof result, 'object'); +for (let key of Object.keys(result)) { + assertEquals(typeof result[key], 'number'); +} From 78c9401cc1802bbb18c3614eaa71cd9578c34b10 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 10 Oct 2022 08:36:43 +0000 Subject: [PATCH 2/2] deps: V8: backport bbd800c6e359 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [heap] Fix incorrect from space committed size NewSpace page operations like RemovePage, PrependPage, and EnsureCurrentCapacity should account for committed page size. This may happen when a page was promoted from the new space to old space on mark-compact. Also, add DCHECKs on Commit and Uncommit to ensure the final committed page size is the same as the current state. Bug: v8:12657 Change-Id: I7aebc1fd3f51f177ae2ef6420f757f0c573e126b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3504766 Reviewed-by: Dominik Inführ Commit-Queue: Chengzhong Wu Cr-Commit-Position: refs/heads/main@{#79426} Refs: https://github.com/v8/v8/commit/bbd800c6e3598a3756733a9c7eba00d7de168226 --- common.gypi | 2 +- deps/v8/src/heap/new-spaces.cc | 17 ++++++++++++++++- deps/v8/test/mjsunit/regress/regress-12657.js | 11 +++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/regress-12657.js diff --git a/common.gypi b/common.gypi index 4e9c78827e6414..43232a5c4b3739 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,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.23', + 'v8_embedder_string': '-node.24', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/heap/new-spaces.cc b/deps/v8/src/heap/new-spaces.cc index d08fe48f23f525..8bc87c647d8d4b 100644 --- a/deps/v8/src/heap/new-spaces.cc +++ b/deps/v8/src/heap/new-spaces.cc @@ -54,6 +54,7 @@ bool SemiSpace::EnsureCurrentCapacity() { // Free all overallocated pages which are behind current_page. while (current_page) { MemoryChunk* next_current = current_page->list_node().next(); + AccountUncommitted(Page::kPageSize); memory_chunk_list_.Remove(current_page); // Clear new space flags to avoid this page being treated as a new // space page that is potentially being swept. @@ -74,6 +75,7 @@ bool SemiSpace::EnsureCurrentCapacity() { NOT_EXECUTABLE); if (current_page == nullptr) return false; DCHECK_NOT_NULL(current_page); + AccountCommitted(Page::kPageSize); memory_chunk_list_.PushBack(current_page); marking_state->ClearLiveness(current_page); current_page->SetFlags(first_page()->GetFlags(), @@ -106,6 +108,7 @@ void SemiSpace::TearDown() { bool SemiSpace::Commit() { DCHECK(!IsCommitted()); + DCHECK_EQ(CommittedMemory(), size_t(0)); const int num_pages = static_cast(target_capacity_ / Page::kPageSize); DCHECK(num_pages); for (int pages_added = 0; pages_added < num_pages; pages_added++) { @@ -134,14 +137,19 @@ bool SemiSpace::Commit() { bool SemiSpace::Uncommit() { DCHECK(IsCommitted()); + int actual_pages = 0; while (!memory_chunk_list_.Empty()) { + actual_pages++; MemoryChunk* chunk = memory_chunk_list_.front(); memory_chunk_list_.Remove(chunk); heap()->memory_allocator()->Free(chunk); } current_page_ = nullptr; current_capacity_ = 0; - AccountUncommitted(target_capacity_); + size_t removed_page_size = + static_cast(actual_pages * Page::kPageSize); + DCHECK_EQ(CommittedMemory(), removed_page_size); + AccountUncommitted(removed_page_size); heap()->memory_allocator()->unmapper()->FreeQueuedChunks(); DCHECK(!IsCommitted()); return true; @@ -246,6 +254,7 @@ void SemiSpace::RemovePage(Page* page) { } } memory_chunk_list_.Remove(page); + AccountUncommitted(Page::kPageSize); for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) { ExternalBackingStoreType t = static_cast(i); DecrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t)); @@ -258,6 +267,7 @@ void SemiSpace::PrependPage(Page* page) { page->set_owner(this); memory_chunk_list_.PushFront(page); current_capacity_ += Page::kPageSize; + AccountCommitted(Page::kPageSize); for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) { ExternalBackingStoreType t = static_cast(i); IncrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t)); @@ -319,6 +329,7 @@ void SemiSpace::Verify() { external_backing_store_bytes[static_cast(i)] = 0; } + int actual_pages = 0; for (Page* page : *this) { CHECK_EQ(page->owner(), this); CHECK(page->InNewSpace()); @@ -344,7 +355,11 @@ void SemiSpace::Verify() { CHECK_IMPLIES(page->list_node().prev(), page->list_node().prev()->list_node().next() == page); + + actual_pages++; } + CHECK_EQ(actual_pages * size_t(Page::kPageSize), CommittedMemory()); + for (int i = 0; i < kNumTypes; i++) { ExternalBackingStoreType t = static_cast(i); CHECK_EQ(external_backing_store_bytes[t], ExternalBackingStoreBytes(t)); diff --git a/deps/v8/test/mjsunit/regress/regress-12657.js b/deps/v8/test/mjsunit/regress/regress-12657.js new file mode 100644 index 00000000000000..e21b7174f8b0e8 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-12657.js @@ -0,0 +1,11 @@ +// Copyright 2022 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --gc-global --expose-statistics --max-semi-space-size=1 + +const a = new Array(); +for (var i = 0; i < 50000; i++) { + a[i] = new Object(); +} +assertTrue(getV8Statistics().new_space_commited_bytes <= 2 * 1024 * 1024);