[cppgc] Don't set object start bit on regular LAB allocations [v8/v8 : main]
0 views
Skip to first unread message
Anton Bikineev (Gerrit)
May 25, 2026, 5:57:35 PM (5 days ago) May 25
to Michael Lippautz, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Anton Bikineev voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 25, 2026, 6:58:19 PM (5 days ago) May 25
to Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Anton Bikineev and Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/speedometer3 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/15024cbe490000
Related details
Attention is currently required from:
- Anton Bikineev
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
May 26, 2026, 4:33:21 AM (5 days ago) May 26
to Anton Bikineev, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Anton Bikineev
Michael Lippautz added 9 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 36 (Latest):
Michael Lippautz . resolved
Generally looks good arleady. Nice optimization!
Commit Message
Line 13, Patchset 36 (Latest):
Since the concurrent marker only needs to perform inner-pointerMichael Lippautz . unresolved
Also incremental
File src/heap/cppgc/heap-page.h
Line 399, Patchset 36 (Latest):
DCHECK_LT(address, reinterpret_cast<ConstAddress>(header) +Michael Lippautz . unresolved
Lets try this as CHECK
Line 374, Patchset 36 (Latest):
DCHECK_LT(address, reinterpret_cast<ConstAddress>(header) +Michael Lippautz . unresolved
CHECK here?
Line 350, Patchset 36 (Latest):
ConstAddress object_end =Michael Lippautz . unresolved
I'd add a comment here explaining that this method may find other headers (before this object) because not all object start bits are initially set. The fixup then makes sure that we indeed find the right one.
Line 68, Patchset 36 (Latest):
template <AccessMode = AccessMode::kNonAtomic>Michael Lippautz . unresolved
I'd add a comment here that these methods assume that the bit exists for address and do not perform any fixups.
Line 62, Patchset 36 (Latest):
// |address| must refer to real object.Michael Lippautz . unresolved
I'd mention here that these methods must run on the main thread as they potentially fix up any missing bits.
File src/heap/cppgc/sweeper.cc
Line 1016, Patchset 36 (Parent):
ObjectStartBitmapVerifier().Verify(heap_);Michael Lippautz . unresolved
Can we verify individual pages after sweeping?
Related details
Attention is currently required from:
- Anton Bikineev
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
May 26, 2026, 6:09:53 AM (5 days ago) May 26
to Michael Lippautz, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Anton Bikineev added 8 comments![]( "Open in Gerrit")
Commit Message
Line 13, Patchset 36:
Since the concurrent marker only needs to perform inner-pointerMichael Lippautz . resolved
Anton BikineevAlso incremental
Done
File src/heap/cppgc/heap-page.h
Line 399, Patchset 36:
DCHECK_LT(address, reinterpret_cast<ConstAddress>(header) +Michael Lippautz . resolved
Lets try this as CHECK
Anton Bikineev
Done
Line 374, Patchset 36:
DCHECK_LT(address, reinterpret_cast<ConstAddress>(header) +Michael Lippautz . resolved
Anton BikineevCHECK here?
Done
I'd add a comment here explaining that this method may find other headers (before this object) because not all object start bits are initially set. The fixup then makes sure that we indeed find the right one.
Anton Bikineev
Done
I'd add a comment here that these methods assume that the bit exists for address and do not perform any fixups.
Anton Bikineev
Done
I'd mention here that these methods must run on the main thread as they potentially fix up any missing bits.
Anton Bikineev
Done
File src/heap/cppgc/sweeper.cc
Line 1016, Patchset 36 (Parent):
ObjectStartBitmapVerifier().Verify(heap_);Michael Lippautz . unresolved
Can we verify individual pages after sweeping?
Anton Bikineev
We do verify it (see line 743). This one is for before sweeping.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 26, 2026, 6:46:06 AM (5 days ago) May 26
to Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Anton Bikineev and Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
😿 Job android-pixel10-perf/blink_perf.css failed.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/15bdc296490000
Related details
Attention is currently required from:
- Anton Bikineev
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
May 26, 2026, 6:57:50 AM (5 days ago) May 26
to Anton Bikineev, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Anton Bikineev
Michael Lippautz voted and added 3 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
3 comments
Patchset-level comments
File src/heap/cppgc/object-allocator.h
Line 235, Patchset 39 (Latest):
NormalPage::From(BasePage::FromPayload(&filler))Michael Lippautz . unresolved
I guess we rarely go into this path here because it requires double-word alignment on the caller side? We could also avoid writing this bit fwiw.
Reading this code, I wonder if it's not slightly better to refactor this a bit into
```
if (kAlignment != kAllocaitonGranularity) {
return AllocateWithCustomAlignment();
}
// Assume single-word alignment here.
```
Feel free to ignore this comment.
File src/heap/cppgc/sweeper.cc
Line 1016, Patchset 36 (Parent):
ObjectStartBitmapVerifier().Verify(heap_);Michael Lippautz . resolved
Anton BikineevCan we verify individual pages after sweeping?
We do verify it (see line 743). This one is for before sweeping.
Michael Lippautz
Ack, thanks. Before sweeping doesn't work anymore.
Related details
Attention is currently required from:
- Anton Bikineev
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
May 26, 2026, 7:26:04 AM (5 days ago) May 26
to Michael Lippautz, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Anton Bikineev added 1 comment![]( "Open in Gerrit")
File src/heap/cppgc/object-allocator.h
I guess we rarely go into this path here because it requires double-word alignment on the caller side? We could also avoid writing this bit fwiw.
Reading this code, I wonder if it's not slightly better to refactor this a bit into
```
if (kAlignment != kAllocaitonGranularity) {
return AllocateWithCustomAlignment();
}
// Assume single-word alignment here.
```Feel free to ignore this comment.
Anton Bikineev
agreed, refactoring could be done in a separate CL.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
May 26, 2026, 8:05:11 AM (5 days ago) May 26
to Michael Lippautz, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Anton Bikineev voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
May 26, 2026, 8:06:44 AM (5 days ago) May 26
to Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
39 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/heap/cppgc/object-allocator.h
Insertions: 1, Deletions: 4.
@@ -231,10 +231,7 @@
if (!lab_allocation_will_succeed &&
(current_lab_size >= (size + kPaddingSize))) {
void* filler_memory = current_lab.Allocate(kPaddingSize);
- auto& filler = Filler::CreateAt(filler_memory, kPaddingSize);
- NormalPage::From(BasePage::FromPayload(&filler))
- ->object_start_bitmap()
- .SetBit<AccessMode::kAtomic>(reinterpret_cast<ConstAddress>(&filler));
+ Filler::CreateAt(filler_memory, kPaddingSize);
lab_allocation_will_succeed = true;
}
if (V8_UNLIKELY(!lab_allocation_will_succeed)) {
```
Change information
Commit message:
[cppgc] Don't set object start bit on regular LAB allocations
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 <biki...@chromium.org>
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107560}
Files:
- M include/cppgc/allocation.h
- M src/handles/traced-handles-inl.h
- M src/heap/cppgc/allocation.cc
- M src/heap/cppgc/heap-page.cc
- M src/heap/cppgc/heap-page.h
- M src/heap/cppgc/object-allocator.cc
- M src/heap/cppgc/object-allocator.h
- M src/heap/cppgc/object-start-bitmap.h
- M src/heap/cppgc/sweeper.cc
- M src/heap/cppgc/trace-trait.cc
Change size: L
Delta: 10 files changed, 321 insertions(+), 126 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 26, 2026, 11:15:14 AM (5 days ago) May 26
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job android-pixel6-perf/speedometer3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/10efe686490000
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 26, 2026, 3:38:37 PM (5 days ago) May 26
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
😿 Job android-pixel10-perf/blink_perf.css failed.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1529ef76490000
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 26, 2026, 5:10:46 PM (5 days ago) May 26
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job android-pixel10-perf/blink_perf.css complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/106a3be9490000
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 26, 2026, 5:51:05 PM (4 days ago) May 26
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job android-pixel10-perf/speedometer3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1221d509490000
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
May 27, 2026, 12:39:05 AM (4 days ago) May 27
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Anton Bikineev, Michael Lippautz, android-bu...@system.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job android-pixel9-perf/blink_perf.css complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/12549c06490000
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages