heap: Make range-based write barrier more robust [chromium/src : main]
0 views
Skip to first unread message
Michael Lippautz (Gerrit)
May 18, 2026, 11:43:22 AM (13 days ago) May 18
to Anton Bikineev, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-...@chromium.org, kinuko...@chromium.org, kouhe...@chromium.org, oilpan-rev...@chromium.org
Attention needed from Anton Bikineev
Michael Lippautz added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Anton Bikineev
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bikineev (Gerrit)
May 19, 2026, 5:25:08 AM (12 days ago) May 19
to Michael Lippautz, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-...@chromium.org, kinuko...@chromium.org, kouhe...@chromium.org, oilpan-rev...@chromium.org
Attention needed from Michael Lippautz
Anton Bikineev voted and added 2 comments![]( "Open in Gerrit")
Votes added by Anton Bikineev
| Code-Review | +1 |
2 comments
Patchset-level comments
Anton Bikineev . resolved
lgtm % nit
File third_party/blink/renderer/platform/heap/member.h
Line 251, Patchset 2 (Latest):
for (auto& member : members) {Anton Bikineev . unresolved
Coudl we merge the loop above into this one?
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Coverage
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 19, 2026, 5:56:29 AM (12 days ago) May 19
to Anton Bikineev, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-...@chromium.org, kinuko...@chromium.org, kouhe...@chromium.org, oilpan-rev...@chromium.org
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Commit-Queue | +2 |
1 comment
File third_party/blink/renderer/platform/heap/member.h
Coudl we merge the loop above into this one?
Michael Lippautz
I merged the loops and left behind a TODO to use a single global variable checks as a fast path here. We need to expose that on V8 first though
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
May 19, 2026, 7:41:12 AM (12 days ago) May 19
to Anton Bikineev, Chromium LUCI CQ, chromium...@chromium.org, Kentaro Hara, blink-...@chromium.org, kinuko...@chromium.org, kouhe...@chromium.org, oilpan-rev...@chromium.org
Michael Lippautz voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
May 19, 2026, 8:34:05 AM (12 days ago) May 19
to Michael Lippautz, Anton Bikineev, chromium...@chromium.org, Kentaro Hara, blink-...@chromium.org, kinuko...@chromium.org, kouhe...@chromium.org, oilpan-rev...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/platform/heap/member.h
Insertions: 16, Deletions: 15.
@@ -230,26 +230,27 @@
}
static void NotifyNewElements(base::span<T> members) {
- // Checking one pointer is sufficient for determining whether a
- // marking or generational barrier is required. We need a non-null pointer
- // to check if the write barrier is needed. nullptr will just always bail
- // out.
- T* first_non_null = [&]() -> T* {
- for (auto& member : members) {
- if (static_cast<bool>(member)) {
- return &member;
- }
+ // TODO(mlippautz): We can expose whether the write barrier is enabled at
+ // all and get away with a single global variable check here.
+
+ // We need a non-null pointer to check if the write barrier is needed.
+ // nullptr will just always bail out.
+ auto current = members.begin();
+ for (; current != members.end(); ++current) {
+ if (static_cast<bool>(*current)) {
+ break;
}
- return nullptr;
- }();
- if (!first_non_null) {
+ }
+ if (current == members.end()) {
return;
}
- if (!WriteBarrier::IsWriteBarrierNeeded(first_non_null)) [[likely]] {
+ // Checking one pointer is sufficient for determining whether a
+ // marking or generational barrier is required.
+ if (!WriteBarrier::IsWriteBarrierNeeded(&*current)) {
return;
}
- for (auto& member : members) {
- WriteBarrier::DispatchForObject(&member);
+ for (; current != members.end(); ++current) {
+ WriteBarrier::DispatchForObject(&*current);
}
}
};
```
Change information
Commit message:
heap: Make range-based write barrier more robust
Fix bailouts on the barrier for possible nullptr cases.
CONV=47326a54-47d2-449d-b308-f51eb6b5b3b8
TAG=AGY
Fixed: 513737952
Change-Id: Iaa8598bc06a1782f77af7a3ea02359d918673688
Reviewed-by: Anton Bikineev <biki...@chromium.org>
Commit-Queue: Michael Lippautz <mlip...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1632789}
Files:
- M third_party/blink/renderer/platform/heap/member.h
- M third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
Change size: M
Delta: 2 files changed, 73 insertions(+), 6 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Anton Bikineev
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages