include/cppgc/heap.h is a public header, and since b3cb54c7e6
("[heap] Add StackStartMarker for cppgc") it calls
__builtin_frame_address(0) unconditionally. That builtin does not exist
in MSVC, so any translation unit built with cl.exe that includes the
header (directly or through v8-cppgc.h) fails to compile, even if it
never constructs a StackStartMarker.
Use _AddressOfReturnAddress() when V8_CC_MSVC is set, which is what
base::Stack::GetCurrentStackPosition() already does for the same
purpose, and keep __builtin_frame_address(0) elsewhere.
Bug: none
Change-Id: Icd4c0238cb277c8e1b73454534ffb2519b302ce9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8361192
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Shelley Vohr <shelley.vohr@gmail.com>
Cr-Commit-Position: refs/heads/main@{#109737}
The most critical barrier (CombinedWriteBarrierSlow) was already
annotated. Add more annotations to cover all the collection use cases
as well.
Speedometer3
👍 TodoMVC-Preact-Complex-DOM [ -1.1%, -0.0%]
Bug: 537244297
Change-Id: Ie9f1823ba99f5e2ca8b15d446c10acf88b302d49
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8126059
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108791}
Place GCInfo objects into a separate section (`gc_info_section` or
`__DATA,gc_info_section`) and compute the index directly as an offset
from the start of the section. This avoids the overhead of dynamic
registration and checking on allocation. This also eliminates the need
for `GCInfoTable` when active.
The optimization is enabled by default on desktop (Linux and macOS)
under the `cppgc_enable_object_section_gcinfo` GN flag for non-component
builds.
ELF and MachO platforms are supported.
TAG=agy
Change-Id: I83b5bf36dd8f0d0f469caf3dc8db4eb9464c2250
Bug: 522693606
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7918333
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107939}
To improve allocation performance, this CL avoids setting the object
start bit in the ObjectStartBitmap during fast-path LAB allocations for
regular (non-mixin) objects.
Since the concurrent/incremental marker only needs to perform
inner-pointer resolution for mixin types, we can safely omit setting
the start bit for non-mixin objects during allocation. The bitmap is
instead repaired on-the-fly by the mutator when needed, and fully
populated during sweeping.
Detailed changes:
1. Allocation Optimization: Removed the `SetBit` call in `ObjectAllocator`
for LAB allocations, except when `may_contain_mixins == kYes`. Mixins
continue to set their start bit immediately to ensure they can be
reliably found by the concurrent marker.
2. On-the-fly Repair (Mutator): `BasePage::ObjectHeaderFromInnerAddress`
now detects if the bitmap has gaps and repairs it on-the-fly by
linearizing the bitmap from the last known set bit up to the requested
address. This is safe as it is only called by the mutator.
3. Concurrent Lookup (Marker): Introduced
`BasePage::ObjectHeaderFromInnerAddressConcurrent` for the concurrent
marker. To avoid data races with the mutator, it strictly reads from
the bitmap and does not attempt to repair it. This is safe because the
concurrent marker only resolves inner pointers for mixin objects, which
are guaranteed to have had their start bits set upon allocation.
4. Sweeper Updates: The sweeper now populates the start bit for all
live objects and free-list entries, restoring the bitmap to a fully
consistent state at the end of the sweep phase.
Design Doc: https://docs.google.com/document/d/1i4Sx-p6a_toYLwEt4HnQeJ9Xm28FmevwT2e5DzkGIaQ/edit?usp=sharing
Bug: 516682115
Change-Id: Ib8a3dfd28778d3b07beb5bf8f38331861b384b6b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7828550
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107560}
Enabling cppgc_enable_object_names causes ASAN false positives
when iterating cross-thread persistents. This is because of the
access to the location_ field. This CL fixes this by silencing
ASAN for this access similar to what we do for GetNodeFromGC() or
ClearFromGC().
Bug: 497855658
Change-Id: I6af31de5b6273a8cfde835388dbf64e826a65ef9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7865927
Auto-Submit: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107520}
This CL initially enables this "gn arg" on Desktop platforms. Android
and Fuchsia can be enabled later as well.
Bug: 497855658
Change-Id: I09696c4df93f403a7eff349a74037e1ee167a557
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7807042
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107492}
EphemeronPair's value is automatically cleared when the key is dead, but
the key is not, which leaves a dangling pointer when the weak key is
GCed. This CL fixes that by clearing the key also.
Bug: chromium:508082779
Change-Id: Icbd3d3ccadb4f12ff89dbac7ceee83f63fe256c4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7817830
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Scott Haseley <shaseley@chromium.org>
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107080}
For conservative stack scanning we currently scan for references up
until the stack top. But this includes stack frames like that of
_start which definitely do not have pointers into the cppgc heap.
This CL adds a StackStartMarker class which embedders can optionally
use to decrease the stack height considered during conservative
stack scanning. This reduces the chance of false positives.
Bug: 492198560
Change-Id: Ic9a7d428684bb835d372873beb821e412e131246
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7666257
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105816}
This reverts commit b284c48c16.
Reason for revert: Performance regressions in event dispatch, presumably from LLVM [[assume]] weaknesses; since we don't see any clear wins anywhere else (besides a bit in size), it's easier to just revert.
Original change's description:
> Make Uncompress(Compress(ptr)) into a no-op.
>
> Add an [[assume]] annotation so that Clang understands this; it doesn't
> by itself (because it requires information about high bits in the
> uncompressed pointer that we've found it hard to teach it).
>
> Doesn't give a Speedometer win by itself, but it reduces the APK size
> of 64-bit Chrome by ~11 kB.
>
> Change-Id: I1480f0032e3733972553b1e34eb4a7def5f157ed
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7277326
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
> Auto-Submit: Steinar H Gunderson <sesse@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#104529}
Fixed: 474389112
Change-Id: I924a6613291a7f422736b3b914e7cc8dd58b2676
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7487369
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104742}
Add an [[assume]] annotation so that Clang understands this; it doesn't
by itself (because it requires information about high bits in the
uncompressed pointer that we've found it hard to teach it).
Doesn't give a Speedometer win by itself, but it reduces the APK size
of 64-bit Chrome by ~11 kB.
Change-Id: I1480f0032e3733972553b1e34eb4a7def5f157ed
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7277326
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
Auto-Submit: Steinar H Gunderson <sesse@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104529}
The GCInfoIndex is always written on allocation. This does not affect
writing/reading the construction bit which happens with a mask.
This safes one shift in the allocation fast path (for GCInfoIndex).
Drive-by: Use std::atomic_ref instead of casts to std::atomic.
Change-Id: I8932f46ef582558d30eded9bab9b8d210a0ee1cb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7213182
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104077}
Enforce that the Visitor::Trace(const T&) method that is used for
non-reference fields (e.g. embedded DISALLOW_NEW objects) is never
called for GCed types.
Change-Id: Id896ff1dcec3c42a7e9bc11c4a64e850d8e7e143
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7046265
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#103236}
The blink contains two type of object: the dom objects have explicitly
specified names via using NameProvider, and other cpp internal objects
have hide names 'InternalNode'. Now we must enable
CPPGC_SUPPORTS_OBJECT_NAMES macro to show the name and size of all blink
objects in memory-infra. I think it is better that display the count and
size of these dom objects inherited from NameProvider without enabling
CPPGC_SUPPORTS_OBJECT_NAMES macro in release version.
It will provide great convenience to help us debug the memory issues of
the blink object.
Bug: 445232188
Change-Id: I436cd08e0ec337f72ac95529f0a73d734d2c0bd3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6950858
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#103057}
SourceLocation is almost only used for debug messages in TurboShaft, so
pass it only in debug mode. Additionally, std::source_location (that it
wraps) is a single pointer, so we can efficiently pass it by value
rather than const ref.
This reduces release build d8 by ~170kb, of which ~4kb is the by-value
passing.
Change-Id: Ia6b592f094dad0987b8a886bf16a7d3fe8611eb1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6888815
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#102072}
The recent change that got rid of clearing HeapVector's inlined storage
introduced an issue for the barrier, which may still invoke the regular
tracing routines for scanning on-stack inlined storages, which may run
into unitialized storage.
The CL introduces a way to distinguish the main-thread visitor (plus the
write barrier) and the concurrent marker. For the main thread marker the
main thread visitor may simply scan inlined storage up until the actual
size, not capacity.
Bug: 417904220
Change-Id: I8caf2ae644fc1ee975958d102414d369595d63e4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6550401
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100307}
This is a reland of commit 3863ce60d3
On Fuchsia, `base::OS::Initialize()`, called by `V8::Initialize()`,
initializes the VMEX resource, which is needed to allocate memory with
the PageAllocator of the default platform. However, the original CL
called `cppgc::InitializeProcess()` before `V8::Initialize()`, and
therefore the allocation of the GCInfoTable failed. Calling
`cppgc::InitializeProcess()` after `V8::Initialize()` fixed the issue.
Original change's description:
> Reland "[cppgc] Automatically set up CppHeap if not specially provided"
>
> This is a reland of commit fe8642e294
>
> The test `GlobalHandlesTest.TotalSizeTracedNode` tested that after a GC,
> traced handles would not exist anymore. However, with CSS a traced
> handle stayed alive. The test gets fixed by opening a
> DisableConservativeStackScanningScopeForTesting scope.
>
> Original change's description:
> > [cppgc] Automatically set up CppHeap if not specially provided
> >
> > It is planned to allow the usage of cppgc in V8. For that it has to be
> > guaranteed that a CppHeap exists for every isolate. So far the CppHeap
> > was provided by the embedder, and if the embedder did not set up a
> > CppHeap, it would not exist, and V8 could not use it. With this CL, a
> > default CppHeap gets created when no CppHeap is provided by the
> > embedder.
Bug: 42203693
Change-Id: Ieb8a11880af1621ed03e8a69e82ce22479be1d32
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6348469
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#99346}
This is a reland of commit 017b2a5132
Original change's description:
> [cppgc] Use blink_gc_plugin in v8
>
> This CL enables the plugin for v8 builds and fixes various violations
> found in the v8 codebase.
>
> Some ignore annotations remain for changes required in the plugin (e.g.
> treat TraceMultiple and TraceEphemeron as trace methods).
>
> Bug: 402564649
> Change-Id: I3f995cf2bf31672b7c5702338286cfb850af2dcf
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6366816
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Commit-Queue: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#99316}
Bug: 402564649
Change-Id: Ibb4c9efdd6f26a9d7de234c4f341b754c96fc345
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6369299
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#99323}
This CL enables the plugin for v8 builds and fixes various violations
found in the v8 codebase.
Some ignore annotations remain for changes required in the plugin (e.g.
treat TraceMultiple and TraceEphemeron as trace methods).
Bug: 402564649
Change-Id: I3f995cf2bf31672b7c5702338286cfb850af2dcf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6366816
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#99316}
This reverts commit 3863ce60d3.
Reason for revert: Suspected to failing the chromium roll:
https://ci.chromium.org/ui/p/chromium/builders/try/fuchsia-x64-cast-receiver-rel/960200/overview
> Fatal process out of memory: Oilpan: GCInfoTable initial
> reservation.
Original change's description:
> Reland "[cppgc] Automatically set up CppHeap if not specially provided"
>
> This is a reland of commit fe8642e294
>
> The test `GlobalHandlesTest.TotalSizeTracedNode` tested that after a GC,
> traced handles would not exist anymore. However, with CSS a traced
> handle stayed alive. The test gets fixed by opening a
> DisableConservativeStackScanningScopeForTesting scope.
>
> Original change's description:
> > [cppgc] Automatically set up CppHeap if not specially provided
> >
> > It is planned to allow the usage of cppgc in V8. For that it has to be
> > guaranteed that a CppHeap exists for every isolate. So far the CppHeap
> > was provided by the embedder, and if the embedder did not set up a
> > CppHeap, it would not exist, and V8 could not use it. With this CL, a
> > default CppHeap gets created when no CppHeap is provided by the
> > embedder.
> >
> > Bug: 42203693
> > Change-Id: I59e04618c1d6cdac8091216117a30519f65d7235
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6190932
> > Commit-Queue: Andreas Haas <ahaas@chromium.org>
> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#99090}
>
> Bug: 42203693
> Change-Id: Id548ef84aecf1d20b3da8c943e9dfa5f694eeaf7
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6333562
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#99191}
Bug: 42203693
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I795d21a27f5bf6ea17448add04d73f25e39eb81c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6346807
Owners-Override: Matthias Liedtke <mliedtke@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#99202}
This is a reland of commit fe8642e294
The test `GlobalHandlesTest.TotalSizeTracedNode` tested that after a GC,
traced handles would not exist anymore. However, with CSS a traced
handle stayed alive. The test gets fixed by opening a
DisableConservativeStackScanningScopeForTesting scope.
Original change's description:
> [cppgc] Automatically set up CppHeap if not specially provided
>
> It is planned to allow the usage of cppgc in V8. For that it has to be
> guaranteed that a CppHeap exists for every isolate. So far the CppHeap
> was provided by the embedder, and if the embedder did not set up a
> CppHeap, it would not exist, and V8 could not use it. With this CL, a
> default CppHeap gets created when no CppHeap is provided by the
> embedder.
>
> Bug: 42203693
> Change-Id: I59e04618c1d6cdac8091216117a30519f65d7235
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6190932
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#99090}
Bug: 42203693
Change-Id: Id548ef84aecf1d20b3da8c943e9dfa5f694eeaf7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6333562
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#99191}
This reverts commit fe8642e294.
Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux/59854/overview
Original change's description:
> [cppgc] Automatically set up CppHeap if not specially provided
>
> It is planned to allow the usage of cppgc in V8. For that it has to be
> guaranteed that a CppHeap exists for every isolate. So far the CppHeap
> was provided by the embedder, and if the embedder did not set up a
> CppHeap, it would not exist, and V8 could not use it. With this CL, a
> default CppHeap gets created when no CppHeap is provided by the
> embedder.
>
> Bug: 42203693
> Change-Id: I59e04618c1d6cdac8091216117a30519f65d7235
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6190932
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#99090}
Bug: 42203693
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I56768116b576e48c657b5461113a9f34db9f764f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6330140
Owners-Override: Manos Koukoutos <manoskouk@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#99091}
It is planned to allow the usage of cppgc in V8. For that it has to be
guaranteed that a CppHeap exists for every isolate. So far the CppHeap
was provided by the embedder, and if the embedder did not set up a
CppHeap, it would not exist, and V8 could not use it. With this CL, a
default CppHeap gets created when no CppHeap is provided by the
embedder.
Bug: 42203693
Change-Id: I59e04618c1d6cdac8091216117a30519f65d7235
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6190932
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#99090}
This fixes the handling of intermediate values when conservatively
iterating memory (stack, but also heap).
To this end we will scan through memory word-by-word and consider:
1. Full pointers;
2. Compressed pointers in both half words;
3. A half-decompressed 35-bit value as intermediate state that doesn't
assume sign extension happened;
We also bring back the optimization from https://crrev.com/c/6198226
which relied on the fact that the full pointer will be reached through
the intermediate state. This is now actually the case and handling 3.
implicitly also handles 1.
The CL also adds a bunch of tests that would have caught the original
problem.
Change-Id: Ibde3af33894b1f044cde49e97492b46c3e4b4e24
Bug: 391018461
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6198347
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#98372}
The 2GB cage was already disabled for some time due to OOM errors. We
have also increased the cage from 4GB to 16GB already on non-Android
systems. So it seems unlikely that we could launch it again.
Bug: 343959927
Change-Id: I27c26256c84f4e592bb991d7b8a0582b54685e18
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6089581
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Auto-Submit: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#97791}
This is a reland of commit 48bdf7ae9f
Original change's description:
> cppgc: Remove guard pages and avoid discarding pages on Windows
>
> This is a reland of https://crrev.com/c/6049213 with improved handling
> for Windows.
>
> This CL changes the guard page size to 0 on all platforms. Individual
> Oilpan pages thus have no more guard regions. The infrastructure will
> be removed in a subsequent CL.
>
> On Windows discarding memory on 128K-aligned 128K in size chunks
> caused massive performance regressions on syscalls.
>
> The CL works around this by removing discarding in the following ways:
> 1. On destroying pages the memory is not discarded anymore. This is
> okay because we only discard pages on memory reducing GCs which is
> also when we drop pages from the pool which decommits the memory
> anyways.
> 2. Discarding free list entries is disabled on Windows. This is in
> line with the JS heap part which has a similar exemption.
>
> Change-Id: I8623467ef06eb3be63cd62718db09dde65e4a763
> Bug: 378017037
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6055482
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#97600}
Cq-Include-Trybots: luci.v8.try:v8_win64_nodcheck_rel
Bug: 378017037
Change-Id: If02d76b37e39eb14949bc7a7769ce2771eaf1eab
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6074477
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#97629}
This reverts commit 48bdf7ae9f.
Reason for revert: Fails on https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Win64/61462/overview
Original change's description:
> cppgc: Remove guard pages and avoid discarding pages on Windows
>
> This is a reland of https://crrev.com/c/6049213 with improved handling
> for Windows.
>
> This CL changes the guard page size to 0 on all platforms. Individual
> Oilpan pages thus have no more guard regions. The infrastructure will
> be removed in a subsequent CL.
>
> On Windows discarding memory on 128K-aligned 128K in size chunks
> caused massive performance regressions on syscalls.
>
> The CL works around this by removing discarding in the following ways:
> 1. On destroying pages the memory is not discarded anymore. This is
> okay because we only discard pages on memory reducing GCs which is
> also when we drop pages from the pool which decommits the memory
> anyways.
> 2. Discarding free list entries is disabled on Windows. This is in
> line with the JS heap part which has a similar exemption.
>
> Change-Id: I8623467ef06eb3be63cd62718db09dde65e4a763
> Bug: 378017037
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6055482
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#97600}
Bug: 378017037
Change-Id: I50949ea0085c60908984cf9489f9eac6fd466db0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6074950
Commit-Queue: Eva Herencsárová <evih@chromium.org>
Owners-Override: Eva Herencsárová <evih@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#97602}
This is a reland of https://crrev.com/c/6049213 with improved handling
for Windows.
This CL changes the guard page size to 0 on all platforms. Individual
Oilpan pages thus have no more guard regions. The infrastructure will
be removed in a subsequent CL.
On Windows discarding memory on 128K-aligned 128K in size chunks
caused massive performance regressions on syscalls.
The CL works around this by removing discarding in the following ways:
1. On destroying pages the memory is not discarded anymore. This is
okay because we only discard pages on memory reducing GCs which is
also when we drop pages from the pool which decommits the memory
anyways.
2. Discarding free list entries is disabled on Windows. This is in
line with the JS heap part which has a similar exemption.
Change-Id: I8623467ef06eb3be63cd62718db09dde65e4a763
Bug: 378017037
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6055482
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#97600}
This reverts commit e56d2eafd0.
Reason for revert: Heavily regresses Windows discarding time.
Original change's description:
> cppgc: Remove guard pages
>
> Guard pages (4K, 16K) were added on certain platforms within the 128K
> logical cppgc page. These regions have been implemented and dragged
> along from the initial version of Oilpan.
>
> For tooling these pages have no real use as we rely on ASAN for
> poisoning unused areas. OOB access for large objects is generally
> trapped on access in bounds checks. Other OOB access has to hit
> exactly the guard pages to trap which is not generally the case.
>
> The mechanism had no relevance for actual security as the sizes were
> small and their positiion was deterministic.
>
> This CL removes the guard page support. This gets us back some virtual
> address space in the cage and slightly makes getting to page handles
> cheaper (over the public API).
>
> Bug: 378017037
> Change-Id: I8bb2c17e2daa4ee76c8a29aeaeb46827948160c3
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6049213
> Reviewed-by: Samuel Groß <saelo@chromium.org>
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#97389}
Bug: 378017037, 381201234, 381120830, 380922994
Change-Id: Ia2b55934891fb176acb3473d15151e184c75d44d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6055557
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#97458}
Guard pages (4K, 16K) were added on certain platforms within the 128K
logical cppgc page. These regions have been implemented and dragged
along from the initial version of Oilpan.
For tooling these pages have no real use as we rely on ASAN for
poisoning unused areas. OOB access for large objects is generally
trapped on access in bounds checks. Other OOB access has to hit
exactly the guard pages to trap which is not generally the case.
The mechanism had no relevance for actual security as the sizes were
small and their positiion was deterministic.
This CL removes the guard page support. This gets us back some virtual
address space in the cage and slightly makes getting to page handles
cheaper (over the public API).
Bug: 378017037
Change-Id: I8bb2c17e2daa4ee76c8a29aeaeb46827948160c3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6049213
Reviewed-by: Samuel Groß <saelo@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#97389}
These checks are currently not run in Blink at all because the debug
macros are different. Move them behind v8_enable_slow_dchecks to allow
some testing and try to later enable them more broadly.
This also fixes the issue that for DEBUG builds the sizeof(Member) is
out of sync between V8 and Blink.
Change-Id: I332fb82ffe8f7bb2b1a9a8fee3630be0b06ce471
Bug: 373485798
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5952933
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#96769}
cppgc's API-level checks have been gated behind `DEBUG` whereas Blink
always passes `_DEBUG` as symbol for debugging/dcheck builds.
Put the checks now behind a separate define that follows the common
practice of pairing a gn flag with a define for external usage.
This CL only enabled `CPPGC_DCHECK()` and leaves more expensive
verificaton for a follow up.
Bug: 373485798
Change-Id: Ic45738bb34569f3046118142350ecf7b6ac4439d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5934873
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#96665}
Currently policies just use `Get()` which will trigger a
decompression, even if the result is then not used in some
configurations.
This is a possible small perf win on some configurations
where the decompression was not optimized away.
Bug: 373485798
Change-Id: I72a4c79652c64f5461790dd5d5d8e54874c7e5bd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5938964
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#96663}
Omer Katz
Shelley Vohr
Michael Lippautz
Anton Bikineev
Dominik Inführ
Leszek Swirski
Matthias Liedtke
Scott Haseley
Steinar H Gunderson
Kevin Wang
Takuto Ikuta
Jakob Kummerow
Anton Bikineev
Andreas Haas
Shu-yu Guo
Manos Koukoutos
Peter Kasting
Eva Herencsárová