Issue 12592 in v8: Increase maximum number of Wasm stores (V8 isolates) to more than 256

[piotr… via monorail]

Status: Untriaged
Owner: piotr...@google.com
CC: s...@chromium.org, ad...@chromium.org, ish...@chromium.org, clem...@chromium.org
Priority: 2
Type: Bug

New issue 12592 by piotr...@google.com: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592

Version: 9.2 to 9.9, even worse in LKGR
OS: Linux
Architecture: x64

This is a regression in terms of scalability.

Prior to 9.2, one could easily create 10,000 isolates in a single process.

Since commit 1f504c36da9bab622072d65f80bbf819576c7d3f (which enabled v8_enable_pointer_compression_shared_cage, shipped in 9.2), the maximum number of isolates in a single process dropped drastically to 512.

Since commit 5351e0e805e6e7081250b5b9a015dcff8157d1dc (which enabled v8_enable_external_code_space, to be shipped in 10.0), the maximum number of isolates in a single process dropped even further to 256.

This is quite limiting for our use case (Proxy-Wasm in Envoy), where we create a separate isolate for each plugin on each worker thread (so with a dozen of plugins on a powerful edge server, this easily exceeds the current limits and results in a crash).

I can easily restore the ability to create 10,000 isolates if I disable both features (v8_enable_pointer_compression_shared_cage=false v8_enable_external_code_space=false), but since Sandbox features require v8_enable_pointer_compression_shared_cage, I'm not sure how future-proof this is.

Also, I'd like to verify that those limits are a conscious choice and not imposed by accident because of to the selected data structures.


What steps will reproduce the problem?

Build the following code that creates Wasm stores (V8 isolates) in a loop:

$ cat wee8stores.cc

#include <fstream>
#include <iostream>

#include "wasm.hh"

using wasm::ownvec;
using wasm::Engine;
using wasm::Store;

static const int kCount = 10000;

int main(int argc, const char* argv[]) {
ownvec<Store> stores = ownvec<Store>::make_uninitialized(kCount);
auto engine = Engine::make();
for (int i = 0; i < kCount; i++) {
stores[i] = Store::make(engine.get());
if (stores[i] != nullptr) {
std::ifstream stat("/proc/self/statm");
std::cout << "> Created WasmVM #" << (i + 1) << ": " << stat.rdbuf();
stat.close();
} else {
std::cout << "> Failed to create WasmVM #" << i + 1 << std::endl;
exit(1);
}
}

std::cout << "> Done." << std::endl;
exit(0);
}

$ clang++ -std=c++14 -lpthread -ldl -Iinclude/ -Ithird_party/wasm-api/ wee8stores.cc out/wee8/obj/libwee8.a -o wee8stores


What is the expected output?

$ ./wee8stores

> Created WasmVM #1: 1086927 3070 2697 3092 0 33334 0
[...]

> Created WasmVM #10000: 10486917884 2205422 2512 2883 0 4032903 0
> Done.


What do you see instead?

$ ./wee8stores

> Created WasmVM #1: 1086927 3070 2697 3092 0 33334 0
[...]
> Created WasmVM #256: 1143817 60076 2764 3155 0 131853 0

<--- Last few GCs --->


<--- JS stacktrace --->


#
# Fatal javascript OOM in GC during deserialization
#

Trace/breakpoint trap

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

[ish… via monorail]

Updates:
Cc: piotr...@google.com
Components: Runtime
Owner: ish...@chromium.org
Status: Assigned

Comment #1 on issue 12592 by ish...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c1

It was expected that commit 1f504c36da9bab622072d65f80bbf819576c7d3f (which enabled v8_enable_pointer_compression_shared_cage, shipped in 9.2) will limit the maximum number of isolates in the process. I don't think we considered thousands of Isolates in one process as a real-world use case.

However, it's unexpected that the commit 5351e0e805e6e7081250b5b9a015dcff8157d1dc (which enabled v8_enable_external_code_space, to be shipped in 10.0) reduced the number of Isolates by half. I'll investigate this.

[piotr… via monorail]

Comment #2 on issue 12592 by piotr...@google.com: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c2

That's unfortunate, consisdering that we've reported related scalability issues previously (see: https://bugs.chromium.org/p/v8/issues/detail?id=9651 and https://bugs.chromium.org/p/v8/issues/detail?id=11017).

Is this something that could be revisited? 256 or even 512 is a bit too little for us, especially with V8 crashing upon reaching that limit instead of returning a failure. While we don't need an infinite number of isolates, around a thousand (say, 128 vCPUs with 8-10 plugins) is quite realistic, so it would be great if we could use 4096 as the upper limit.

[s… via monorail]

Updates:
Cc: sa...@chromium.org

Comment #3 on issue 12592 by s...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c3

Piotr, would your use case want to take advantage of JS or WasmGC shared memory features in the future? The v8_enable_pointer_compression_shared_cage feature is a requirement for sharing pointers to GC things between Isolates in the same pointer compression cage.

It should be possible to expose the concept of the pointer compression cage to API and make the embedder choose which Isolates go into which pointer compression cage. You would still have the limit of 512 Isolates per cage, but you'd also be able to make multiple cages per process if you so wish (up to virtual memory exhaustion for the process). Would something like that work for you?

I can't speak to V8 Sandbox project's requirements, so there might be additional requirements there. /cc saelo@ for the Sandbox aspect.

[piotr… via monorail]

Updates:
Cc: jkum...@chromium.org

Comment #4 on issue 12592 by piotr...@google.com: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c4

That's a good question. We use Wasm directly via Wasm C/C++ API (not Embedder API) without any JavaScript, so it's unclear if those features provide any benefits for us. Pointer compression claims both memory and performance improvements (https://v8.dev/blog/pointer-compression), and I believe that's the case for both JS and Wasm, but it's unclear which of the incremental features apply to JS only, and not to Wasm.

My biggest worry with turning default features off is that we'd be using a custom and less tested build, which not always work (e.g. you cannot build V8 with v8_enable_external_code_space=false right now, even though there is no assert gating it on v8_enable_pointer_compression_shared_cage), and those flags are eventually going to go away, at which point we won't be able to disable them anyway.

The original idea behind wee8 was to have a WebAssembly-only build of V8. This never materialized, but perhaps it would make sense to have an officially supported WebAssembly-optimized build in terms of feature flags, so that "v8_wasm_only=true" would automatically turn off some of the JS-only optimizations and other flags targeting browser usage? cc jkummerow@.

As for multiple cages with 512 isolates limit, I think that would work. We'd create a cage per worker thread, and then share it across all the plugins in that thread.

[ish… via monorail]

Comment #5 on issue 12592 by ish...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c5

I think for the Wasm C/C++ API use case the pointer compression doesn't give you any benefits as it mostly affects JavaScript side but not the Wasm.
We do support and test configurations with disabled pointer compression although I agree that the test coverage is not that extensive as with the default configuration.

The V8 Sandbox is also aimed at protecting JavaScript side but not the standalone Wasm side.

TL;DR;: For your use case I'd suggest to disable pointer compression altogether.

I looked at the example and the number of Isolates limitation comes from the number of times we can deserialize the V8 snapshot into the shared heap. Currently the most limiting factor is the number of trampoline Code objects we can create for V8 builtins in the shared code range. Before we enabled v8_enable_external_code_space by default some Code objects were allocated in read-only heap and thus outside the shared code range.

We will address this last regression, however the main question remains - does "v8_wasm_only=true" mode require all the JS features? Maybe the way to go would be to remove all the unnecessary JS builtins and JS objects from the V8 snapshot and thus make each V8 Isolate more lightweight.

[piotr… via monorail]

Comment #6 on issue 12592 by piotr...@google.com: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c6

Pointer compression has some impact on the maximum number of isolates when using trap handlers:
- with PC: max. 7,847 isolates, next allocation results in a crash (Fatal process out of memory: Failed to reserve memory for Isolate V8 pointer compression cage),
- without PC: max. 6,479 isolates, next allocation returns nullptr instead of a crash, allowing for a clean failure.

The 21% drop is quite significant, but that limit is still acceptable, so I might switch to it to prevent V8 from crashing. Thanks for the suggestion!

As for "v8_wasm_only=true" requiring JS builtins, I don't know enough about V8's internals to be able to answer that, but looking at the Wasm C/C++ API implementation, I see that it's using i::JSArrayBuffer, i::JSFunction, i::JSObject, etc.

[ish… via monorail]

Comment #7 on issue 12592 by ish...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c7

As a follow-up to ideas in #c3 we could also add an API option to create a "lightweight" Isolate - with a smaller reservation for pointer compression cage.

Another way to address the code range exhaustion might be to implement lazy/partial deserialization from the V8 snapshot. The "v8_wasm_only=true" use case seems to be another "client" for such a feature.

[sa… via monorail]

Comment #8 on issue 12592 by sa...@google.com: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c8

TL;DR I don't think the sandbox is relevant for your use case.

The V8 Sandbox requires the shared pointer compression cage and can probably only contain exactly one of those. In theory, you could create multiple sandboxes per process, although you might run out of virtual address space fairly quickly since each sandbox currently needs 1TB (although that'd be configurable). However, the sandbox is mostly meant to be useful in a scenario where attackers can supply arbitrary JavaScript code to V8 (i.e., in a web browser), and so can exploit vulnerabilities in V8 itself. IIUC, that's not a scenario you have. I'd assume that we will need to support a non-sandbox configuration anyway, so that'd probably be the way to go here (as ishell@ already suggested above).

[ad… via monorail]

Updates:
Cc: hpa...@chromium.org

Comment #9 on issue 12592 by ad...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c9

(No comment was entered for this change.)

[s… via monorail]

Comment #10 on issue 12592 by s...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c10

> I think for the Wasm C/C++ API use case the pointer compression doesn't give you any benefits as it mostly affects JavaScript side but not the Wasm.

This is definitely true today, so I also agree with ishell@'s suggestion of turning off pointer compression for now. Looking forward to a WasmGC future, that decision might need to be revisited. But that's still a ways off.

[jkumm… via monorail]

Comment #11 on issue 12592 by jkum...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c11

Just to chime in on a (still hypothetical) "v8_wasm_only=true" build configuration: creating that is still on the long-term to-do list, or maybe rather: wishlist. Improving support for use cases of the Wasm C/C++ API is currently not considered a priority by the powers that be in our team. We'll keep it working if bugs are discovered, but there's currently nobody spending (nor planning to spend) time on improvements.
I suppose if someone did want to spend time on it, a fairly quick first step would be to introduce such a GN arg and have it imply other recommended/required GN options, which would at least improve convenience. Untangling V8 internals to actually exclude JS-only stuff from the wee8 build would be a far more involved project.

[Git Watcher via monorail]

Comment #12 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c12

The following revision refers to this bug:
https://chromium.googlesource.com/v8/v8/+/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda

commit 1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda
Author: ish...@chromium.org <ish...@chromium.org>
Date: Mon Aug 08 11:40:32 2022

[ext-code-space] Add InterpreterEntryTrampolineForProfiling builtin

... - a code range size agnostic version of InterpreterEntryTrampoline
builtin. The new builtin is fully compatible with the default version
and used as a template for creating interpreter entry trampoline
Code objects when --interpreted-frames-native-stack is enabled.

This CL introduces a new assembler option "position_independent_code"
which affects the way builtin calls are generated.
This mode is enabled only for InterpreterEntryTrampolineForProfiling.

Motivation:

* InterpreterEntryTrampoline uses RelocInfo::CODE_TARGET for calling
other builtins which requires the code range to be small enough to
allow PC-relative jumps/calls between Code objects. This is the
reason why --interpreted-frames-native-stack was not supported on
arm and might not work on arm64 because the code range is bigger
than the max PC-relative distance for call/jump instructions.
The new builtin calls other builtins via builtins entry table which
makes the code fully relocatable and usable for any code range size.

* RelocInfo::CODE_TARGET requires a target code to be materialized
as a Code object which contradicts the Code-less builtins goal.

* The --interpreted-frames-native-stack is rarely used in the wild but
we have to pay the price of deserializing InterpreterEntryTrampoline
builtin as a Code object which consumes address space in the code
range and thus limits the number of V8 isolates that can be created
because of code range exhaustion. Now the pointer compression cage
becomes the limiting factor instead of the code range.

* We can remove complicated logic of Factory::CopyCode() and respective
support on GC side.

Bug: v8:11880, v8:8713, v8:12592
Change-Id: Ib72e28c03496c43db42f6fe46622def12e102f31
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3811287
Reviewed-by: Jakob Linke <jgr...@chromium.org>
Commit-Queue: Igor Sheludko <ish...@chromium.org>
Reviewed-by: Dominik Inführ <dinf...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82263}

[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/test/cctest/test-serialize.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/compiler.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/heap/setup-heap-internal.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/x64/builtins-x64.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/snapshot/startup-serializer.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/setup-builtins-internal.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/heap/factory.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/arm64/builtins-arm64.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/execution/isolate.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/test/unittests/logging/log-unittest.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/heap/heap.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/arm/builtins-arm.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/builtins-definitions.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/ia32/macro-assembler-ia32.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/snapshot/serializer.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/roots/roots.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/loong64/builtins-loong64.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/snapshot/embedded/embedded-data.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/builtins-interpreter-gen.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/ppc/builtins-ppc.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/flags/flag-definitions.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/ia32/macro-assembler-ia32.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/s390/builtins-s390.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/builtins.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/heap/factory.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/assembler.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/mips64/builtins-mips64.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/test/cctest/heap/test-heap.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/heap/heap.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/diagnostics/disassembler.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/mips/builtins-mips.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/snapshot/embedded/embedded-data-inl.h
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/ia32/builtins-ia32.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/arm64/macro-assembler-arm64.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/snapshot/code-serializer.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/x64/macro-assembler-x64.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/codegen/arm/macro-assembler-arm.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/riscv/builtins-riscv.cc
[modify] https://crrev.com/1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda/src/builtins/builtins.cc

[Git Watcher via monorail]

Comment #13 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c13

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9

commit 725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9
Author: Liu Yu <li...@loongson.cn>
Date: Tue Aug 09 11:17:32 2022

[loong64][mips64][ext-code-space] Add InterpreterEntryTrampolineForProfiling builtin

Port commit 1067c6accc3b57ec5ef4bd4e9b362e0b2feb3bda
Port commit 00746406cfbbd4e838840c20dbe5a3760e3b3f15

Bug: v8:11880, v8:8713, v8:12592

Change-Id: I8787ca38ed8c743f0ee74b2fbd9308b9c8bcb903
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3821204
Commit-Queue: Liu Yu <li...@loongson.cn>
Auto-Submit: Liu Yu <li...@loongson.cn>
Reviewed-by: Zhao Jiazhong <zhaojia...@loongson.cn>
Cr-Commit-Position: refs/heads/main@{#82466}

[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/baseline/mips64/baseline-compiler-mips64-inl.h
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/codegen/loong64/macro-assembler-loong64.h
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/baseline/loong64/baseline-assembler-loong64-inl.h
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/codegen/mips64/macro-assembler-mips64.cc
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/codegen/mips64/macro-assembler-mips64.h
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/baseline/loong64/baseline-compiler-loong64-inl.h
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/codegen/loong64/macro-assembler-loong64.cc
[modify] https://crrev.com/725bdbb3ef62932d4d1c07ae0cf65cdc25c87de9/src/baseline/mips64/baseline-assembler-mips64-inl.h

[Git Watcher via monorail]

Comment #14 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c14

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/40901824d7bf62be97ed8065dc205ed2aa517f36

commit 40901824d7bf62be97ed8065dc205ed2aa517f36
Author: ish...@chromium.org <ish...@chromium.org>
Date: Wed Aug 17 11:07:11 2022

[ext-code-space] Enable Code-less embedded builtins

Bug: v8:11880, v8:12592
Change-Id: I8d3d6ad0a4c26eb1fea2a998ffeddd1d96afa690
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784593
Commit-Queue: Igor Sheludko <ish...@chromium.org>
Reviewed-by: Jakob Linke <jgr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82642}

[modify] https://crrev.com/40901824d7bf62be97ed8065dc205ed2aa517f36/src/common/globals.h

[Git Watcher via monorail]

Comment #15 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c15

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/2f1376409b2eef023af3367aa1975ceaf8fa9ac8

commit 2f1376409b2eef023af3367aa1975ceaf8fa9ac8
Author: Leszek Swirski <les...@chromium.org>
Date: Tue Aug 23 09:23:22 2022

Revert "[ext-code-space] Enable Code-less embedded builtins"

This reverts commit 40901824d7bf62be97ed8065dc205ed2aa517f36.

Reason for revert: UBSan errors (https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20UBSan/22822/overview)

Original change's description:

> [ext-code-space] Enable Code-less embedded builtins
>
> Bug: v8:11880, v8:12592
> Change-Id: I8d3d6ad0a4c26eb1fea2a998ffeddd1d96afa690
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784593
> Commit-Queue: Igor Sheludko <ish...@chromium.org>
> Reviewed-by: Jakob Linke <jgr...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82642}

Bug: v8:11880, v8:12592

Change-Id: Iaf0f87d2e5c1e1e3876d3edc6a82c8578eed81bb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3846471
Auto-Submit: Leszek Swirski <les...@chromium.org>
Commit-Queue: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#82646}

[modify] https://crrev.com/2f1376409b2eef023af3367aa1975ceaf8fa9ac8/src/common/globals.h

[Git Watcher via monorail]

Comment #16 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c16

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/c89998d9adea3612a93c27f7f2f30a4fdf206ff7

commit c89998d9adea3612a93c27f7f2f30a4fdf206ff7

Author: ish...@chromium.org <ish...@chromium.org>
Date: Wed Aug 17 11:07:11 2022

Reland "[ext-code-space] Enable Code-less embedded builtins"

This is a reland of commit 40901824d7bf62be97ed8065dc205ed2aa517f36
The Ubsan issue is fixed here: https://chromium-review.googlesource.com/c/v8/v8/+/3849038.

Original change's description:
> [ext-code-space] Enable Code-less embedded builtins
>
> Bug: v8:11880, v8:12592
> Change-Id: I8d3d6ad0a4c26eb1fea2a998ffeddd1d96afa690
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784593
> Commit-Queue: Igor Sheludko <ish...@chromium.org>
> Reviewed-by: Jakob Linke <jgr...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82642}

Bug: v8:11880, v8:12592

Change-Id: I66373d6af30b060d1204b952d733e260228548df
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3846493

Commit-Queue: Igor Sheludko <ish...@chromium.org>
Reviewed-by: Jakob Linke <jgr...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#82683}

[modify] https://crrev.com/c89998d9adea3612a93c27f7f2f30a4fdf206ff7/src/common/globals.h

[ish… via monorail]

Updates:
Status: Fixed

Comment #17 on issue 12592 by ish...@chromium.org: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c17

With the change in #c16 it's now possible to create up to 2730 of V8 isolates with enabled pointer compression until the address space in the pointer compression cage is fully exhausted by allocations of pages for V8 heaps.

[Git Watcher via monorail]

Comment #18 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c18

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/8809950ef32f0e45a8ad98326eaeb81cf61ae177

commit 8809950ef32f0e45a8ad98326eaeb81cf61ae177
Author: Igor Sheludko <ish...@chromium.org>
Date: Thu Aug 25 10:39:32 2022

Revert "Reland "[ext-code-space] Enable Code-less embedded builtins""

This reverts commit c89998d9adea3612a93c27f7f2f30a4fdf206ff7.

Reason for revert: a lot of unexpected memory regressions that
require investigation.

Original change's description:

> Reland "[ext-code-space] Enable Code-less embedded builtins"
>
> This is a reland of commit 40901824d7bf62be97ed8065dc205ed2aa517f36
> The Ubsan issue is fixed here: https://chromium-review.googlesource.com/c/v8/v8/+/3849038.
>
> Original change's description:
> > [ext-code-space] Enable Code-less embedded builtins
> >
> > Bug: v8:11880, v8:12592
> > Change-Id: I8d3d6ad0a4c26eb1fea2a998ffeddd1d96afa690
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784593
> > Commit-Queue: Igor Sheludko <ish...@chromium.org>
> > Reviewed-by: Jakob Linke <jgr...@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#82642}
>
> Bug: v8:11880, v8:12592
> Change-Id: I66373d6af30b060d1204b952d733e260228548df
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3846493
> Commit-Queue: Igor Sheludko <ish...@chromium.org>
> Reviewed-by: Jakob Linke <jgr...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82683}

Bug: v8:11880, v8:12592, chromium:1356329
Change-Id: I07aaf714da8d7afc66cf4116f189dd5dde5b8818
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3856357
Reviewed-by: Igor Sheludko <ish...@chromium.org>
Commit-Queue: Igor Sheludko <ish...@chromium.org>
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#82721}

[modify] https://crrev.com/8809950ef32f0e45a8ad98326eaeb81cf61ae177/src/common/globals.h

[Git Watcher via monorail]

Comment #19 on issue 12592 by Git Watcher: Increase maximum number of Wasm stores (V8 isolates) to more than 256
https://bugs.chromium.org/p/v8/issues/detail?id=12592#c19

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8/+/91637c25fcdb199526a7060ceca858aeef16bd3d

commit 91637c25fcdb199526a7060ceca858aeef16bd3d

Author: ish...@chromium.org <ish...@chromium.org>
Date: Wed Aug 17 11:07:11 2022

Reland^2 "[ext-code-space] Enable Code-less embedded builtins"

This is a reland of commit 40901824d7bf62be97ed8065dc205ed2aa517f36

The reason for revert was regressions in `blink_gc:effective_size`
buckets of `system_health.memory_desktop` benchmarks.
See http://crbug/1356329#c51.

Memory Perf Sheriffs: This CL shifts GC times which regresses
`blink_gc:effective_size` but improves `v8:effective_size` bucket by
a similar amount. The `private_footprint_size` metric stays neutral
for the majority of the stories and for certain load stories it even
improves.

Original change's description:
> [ext-code-space] Enable Code-less embedded builtins
>
> Bug: v8:11880, v8:12592
> Change-Id: I8d3d6ad0a4c26eb1fea2a998ffeddd1d96afa690
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3784593
> Commit-Queue: Igor Sheludko <ish...@chromium.org>
> Reviewed-by: Jakob Linke <jgr...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82642}

Bug: v8:11880, v8:12592, chromium:1356329, chromium:1356763
Change-Id: Ia9150ecb1f16581e249e4e3e566be20ac4591e78
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3862503
Auto-Submit: Igor Sheludko <ish...@chromium.org>
Reviewed-by: Jakob Linke <jgr...@chromium.org>
Commit-Queue: Jakob Linke <jgr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82803}

[modify] https://crrev.com/91637c25fcdb199526a7060ceca858aeef16bd3d/src/common/globals.h