From e972ff7b13ec2c4ed921fc001dceb5065696e284 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 10 Oct 2022 08:36:43 +0000 Subject: [PATCH] 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 PR-URL: https://github.com/nodejs/node/pull/44947 Refs: https://github.com/v8/v8/commit/b953542909416a5be11220d9adf6da1aff1f009c Reviewed-By: Michaël Zasso Reviewed-By: Jiawen Geng Reviewed-By: Richard Lau --- 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 3adb06c9e0b9c9..cbf9c3810d7400 100644 --- a/common.gypi +++ b/common.gypi @@ -37,7 +37,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);