[cppgc] Fix range DCHECK for marked bytes [v8/v8 : main]
0 views
Skip to first unread message
Michael Lippautz (Gerrit)
Jun 25, 2024, 5:59:26 AM (4 days ago) Jun 25
to Omer Katz, Hannes Payer, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Omer Katz
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Omer Katz
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Jun 25, 2024, 6:04:19 AM (4 days ago) Jun 25
to Michael Lippautz, V8 LUCI CQ, Hannes Payer, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Omer Katz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Omer Katz
| Code-Review | +1 |
2 comments
Patchset-level comments
File src/heap/cppgc/heap-page.h
Line 105, Patchset 2 (Latest):
marked_bytes_.fetch_add(value, std::memory_order_relaxed);Omer Katz . unresolved
nit: You could also write this as:
```
const size_t old_marked_bytes = marked_bytes_.fetch_add(value, std::memory_order_relaxed);
DCHECK_GE(old_marked_bytes + value, old_marked_bytes);
```
I don't think it matters if you crash before or after setting the value.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Jun 25, 2024, 6:04:59 AM (4 days ago) Jun 25
to Omer Katz, V8 LUCI CQ, Hannes Payer, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Michael Lippautz voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Jun 25, 2024, 6:05:09 AM (4 days ago) Jun 25
to Omer Katz, V8 LUCI CQ, Hannes Payer, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Michael Lippautz removed a vote from this change![]( "Open in Gerrit")
Removed Commit-Queue+2 by Michael Lippautz <mlip...@chromium.org>
Related details
Attention set is empty
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Jun 25, 2024, 6:09:53 AM (4 days ago) Jun 25
to Omer Katz, V8 LUCI CQ, Hannes Payer, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Commit-Queue | +2 |
1 comment
File src/heap/cppgc/heap-page.h
Line 105, Patchset 2:
marked_bytes_.fetch_add(value, std::memory_order_relaxed);Omer Katz . resolved
nit: You could also write this as:
```
const size_t old_marked_bytes = marked_bytes_.fetch_add(value, std::memory_order_relaxed);
DCHECK_GE(old_marked_bytes + value, old_marked_bytes);
```
I don't think it matters if you crash before or after setting the value.
Michael Lippautz
Done
Related details
Attention set is empty
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Jun 25, 2024, 6:11:30 AM (4 days ago) Jun 25
to Michael Lippautz, Omer Katz, Hannes Payer, mlippau...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
V8 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: src/heap/cppgc/heap-page.h
Insertions: 2, Deletions: 4.
@@ -97,12 +97,10 @@
size_t discarded_memory() const { return discarded_memory_; }
void IncrementMarkedBytes(size_t value) {
-#ifdef DEBUG
const size_t old_marked_bytes =
- marked_bytes_.load(std::memory_order_relaxed);
+ marked_bytes_.fetch_add(value, std::memory_order_relaxed);
+ USE(old_marked_bytes);
DCHECK_GE(old_marked_bytes + value, old_marked_bytes);
-#endif
- marked_bytes_.fetch_add(value, std::memory_order_relaxed);
}
void ResetMarkedBytes() { marked_bytes_.store(0, std::memory_order_relaxed); }
size_t marked_bytes() const {
```
Change information
Commit message:
[cppgc] Fix range DCHECK for marked bytes
No-try: true
Bug: 333981063
Change-Id: Idc50cccbda5008156286b8a5b4a4733002d6ae56
Reviewed-by: Omer Katz <omer...@chromium.org>
Commit-Queue: Michael Lippautz <mlip...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#94625}
Files:
- M src/heap/cppgc/heap-page.h
Change size: XS
Delta: 1 file changed, 4 insertions(+), 2 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Omer Katz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages