Skip to content

Commit

Permalink
deps: V8: backport bbd800c6e359
Browse files Browse the repository at this point in the history
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 <dinfuehr@chromium.org>
    Commit-Queue: Chengzhong Wu <legendecas@gmail.com>
    Cr-Commit-Position: refs/heads/main@{#79426}

Refs: v8/v8@bbd800c
PR-URL: nodejs/node#44947
Refs: v8/v8@b953542
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
  • Loading branch information
legendecas authored and guangwong committed Jan 3, 2023
1 parent 9fe6023 commit b34417b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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 #####

Expand Down
17 changes: 16 additions & 1 deletion deps/v8/src/heap/new-spaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(),
Expand Down Expand Up @@ -106,6 +108,7 @@ void SemiSpace::TearDown() {

bool SemiSpace::Commit() {
DCHECK(!IsCommitted());
DCHECK_EQ(CommittedMemory(), size_t(0));
const int num_pages = static_cast<int>(target_capacity_ / Page::kPageSize);
DCHECK(num_pages);
for (int pages_added = 0; pages_added < num_pages; pages_added++) {
Expand Down Expand Up @@ -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<MemoryAllocator::kPooledAndQueue>(chunk);
}
current_page_ = nullptr;
current_capacity_ = 0;
AccountUncommitted(target_capacity_);
size_t removed_page_size =
static_cast<size_t>(actual_pages * Page::kPageSize);
DCHECK_EQ(CommittedMemory(), removed_page_size);
AccountUncommitted(removed_page_size);
heap()->memory_allocator()->unmapper()->FreeQueuedChunks();
DCHECK(!IsCommitted());
return true;
Expand Down Expand Up @@ -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<ExternalBackingStoreType>(i);
DecrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
Expand All @@ -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<ExternalBackingStoreType>(i);
IncrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
Expand Down Expand Up @@ -319,6 +329,7 @@ void SemiSpace::Verify() {
external_backing_store_bytes[static_cast<ExternalBackingStoreType>(i)] = 0;
}

int actual_pages = 0;
for (Page* page : *this) {
CHECK_EQ(page->owner(), this);
CHECK(page->InNewSpace());
Expand All @@ -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<ExternalBackingStoreType>(i);
CHECK_EQ(external_backing_store_bytes[t], ExternalBackingStoreBytes(t));
Expand Down
11 changes: 11 additions & 0 deletions deps/v8/test/mjsunit/regress/regress-12657.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit b34417b

Please sign in to comment.