[runtime] Optimize JSObject creation from JSON builder [v8/v8 : main]
0 views
Skip to first unread message
Michael Lippautz (Gerrit)
Sep 4, 2025, 8:48:14 AM (3 days ago) Sep 4
to Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Michael Lippautz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Commit-Queue | +1 |
2 comments
Patchset-level comments
File src/heap/factory.h
Line 1390, Patchset 4 (Latest):
std::optional<Tagged<Object>> maybe_properties,Michael Lippautz . unresolved
Not sure this is the preferred style for this.
Related details
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Sep 4, 2025, 8:57:50 AM (3 days ago) Sep 4
to Michael Lippautz, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Leszek Swirski voted and added 3 comments![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Code-Review | +1 |
3 comments
File src/heap/factory.h
Not sure this is the preferred style for this.
Leszek Swirski
this is good
File src/heap/factory.cc
Line 3149, Patchset 4:
if (V8_LIKELY(!maybe_properties.has_value())) {Leszek Swirski . unresolved
the implicit skipping of the write barrier is that expensive? The faster RO check doesn't help?
File src/objects/js-objects-inl.h
Line 599, Patchset 4:
if (new_js_object_type != NewJSObjectType::kNoEmbedderFieldsAndNoApiWrapper) {
// embedder data slots need to be initialized separately
if (MayHaveEmbedderFields(map)) {Leszek Swirski . unresolved
how about merging these two `ifs`? that way we keep the DCHECK in the `else` and don't need the `DCHECK_IMPLIES` above.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 4, 2025, 9:24:45 AM (3 days ago) Sep 4
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream2 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/131d7e64510000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 4, 2025, 9:27:39 AM (3 days ago) Sep 4
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from 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/111d7e64510000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 4, 2025, 9:43:25 AM (3 days ago) Sep 4
to chrom...@appspot.gserviceaccount.com, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Michael Lippautz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Commit-Queue | +1 |
2 comments
File src/heap/factory.cc
Line 3149, Patchset 4:
if (V8_LIKELY(!maybe_properties.has_value())) {Leszek Swirski . unresolved
the implicit skipping of the write barrier is that expensive? The faster RO check doesn't help?
Michael Lippautz
The way the barrier is set up right now gets at least the IsMarking() flag upfront, which is totally unnecessary. I actually have started some work in this area (https://chromium-review.googlesource.com/c/v8/v8/+/6895242) but it's unclear how the end structure will look like as the barrier could just always look at "pointer from/to here are interesting" which will include RO (but require the page load).
File src/objects/js-objects-inl.h
Line 599, Patchset 4:
if (new_js_object_type != NewJSObjectType::kNoEmbedderFieldsAndNoApiWrapper) {
// embedder data slots need to be initialized separately
if (MayHaveEmbedderFields(map)) {Leszek Swirski . unresolved
how about merging these two `ifs`? that way we keep the DCHECK in the `else` and don't need the `DCHECK_IMPLIES` above.
Michael Lippautz
I also wantto check that we never pass `kNoEmbedderFieldsAndNoApiWrapper` when there could be an embeddeder field which is different from `GetEmbedderFieldCount() == 0`.
Unfortunately (historic reasons...) we can have `MayHaveEmbedderFields(map) == true && GetEmbedderFieldCount(map) == 0` due to stuff being dependend on GN args.
Related details
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Sep 4, 2025, 9:58:37 AM (3 days ago) Sep 4
to Michael Lippautz, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Leszek Swirski voted and added 2 comments![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Code-Review | +1 |
2 comments
File src/heap/factory.cc
Line 3149, Patchset 4:
if (V8_LIKELY(!maybe_properties.has_value())) {Leszek Swirski . resolved
Michael Lippautzthe implicit skipping of the write barrier is that expensive? The faster RO check doesn't help?
The way the barrier is set up right now gets at least the IsMarking() flag upfront, which is totally unnecessary. I actually have started some work in this area (https://chromium-review.googlesource.com/c/v8/v8/+/6895242) but it's unclear how the end structure will look like as the barrier could just always look at "pointer from/to here are interesting" which will include RO (but require the page load).
Leszek Swirski
Acknowledged
File src/objects/js-objects-inl.h
Line 599, Patchset 4:
if (new_js_object_type != NewJSObjectType::kNoEmbedderFieldsAndNoApiWrapper) {
// embedder data slots need to be initialized separately
if (MayHaveEmbedderFields(map)) {Leszek Swirski . resolved
Michael Lippautzhow about merging these two `ifs`? that way we keep the DCHECK in the `else` and don't need the `DCHECK_IMPLIES` above.
I also wantto check that we never pass `kNoEmbedderFieldsAndNoApiWrapper` when there could be an embeddeder field which is different from `GetEmbedderFieldCount() == 0`.
Unfortunately (historic reasons...) we can have `MayHaveEmbedderFields(map) == true && GetEmbedderFieldCount(map) == 0` due to stuff being dependend on GN args.
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 4, 2025, 11:15:09 AM (3 days ago) Sep 4
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from 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/145b7e64510000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 4, 2025, 11:16:11 AM (3 days ago) Sep 4
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream2 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/125b7e64510000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 4, 2025, 11:54:45 AM (3 days ago) Sep 4
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from 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/12c96f39910000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 4, 2025, 12:45:53 PM (3 days ago) Sep 4
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream2 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1558a739910000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 5, 2025, 6:06:17 AM (2 days ago) Sep 5
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream2 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1174bf39910000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 5, 2025, 6:37:23 AM (2 days ago) Sep 5
to chrom...@appspot.gserviceaccount.com, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski and Michael Lippautz
Michael Lippautz removed a vote from this change![]( "Open in Gerrit")
Attention is currently required from:
- Leszek Swirski
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 5, 2025, 6:37:36 AM (2 days ago) Sep 5
to chrom...@appspot.gserviceaccount.com, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 11 (Latest):
Michael Lippautz . resolved
Removing +1, here's the delta: https://chromium-review.googlesource.com/c/v8/v8/+/6915239/6..11
Related details
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Sep 5, 2025, 7:18:16 AM (2 days ago) Sep 5
to Michael Lippautz, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Leszek Swirski voted and added 1 comment![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Code-Review | +1 |
1 comment
File src/objects/js-objects-inl.h
Line 633, Patchset 11:
FillMemoryTaggedUntil(size, filler_map);Leszek Swirski . unresolved
if you want to optimize this further, you could probably use static roots here instead of reading off the roots.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 5, 2025, 7:29:45 AM (2 days ago) Sep 5
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream2 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/10c188b9910000
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 5, 2025, 8:08:54 AM (2 days ago) Sep 5
to Leszek Swirski, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Michael Lippautz added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File src/objects/js-objects-inl.h
Line 633, Patchset 11:
FillMemoryTaggedUntil(size, filler_map);Leszek Swirski . resolved
if you want to optimize this further, you could probably use static roots here instead of reading off the roots.
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Sep 5, 2025, 8:39:16 AM (2 days ago) Sep 5
to Michael Lippautz, Leszek Swirski, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski and Michael Lippautz
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream2 complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/116588b9910000
Related details
Attention is currently required from:
- Leszek Swirski
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Sep 5, 2025, 8:52:38 AM (2 days ago) Sep 5
to Michael Lippautz, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Leszek Swirski voted and added 1 comment![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Code-Review | +1 |
1 comment
File src/objects/slots-inl.h
Line 494, Patchset 14 (Latest):
void MemsetTagged(Tagged_t* start, Tagged<MaybeObject> value, size_t count) {Leszek Swirski . unresolved
nit: this could call the Tagged_t overload now.
Related details
Attention is currently required from:
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 5, 2025, 9:13:25 AM (2 days ago) Sep 5
to Leszek Swirski, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@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
Patchset-level comments
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 5, 2025, 9:13:32 AM (2 days ago) Sep 5
to Leszek Swirski, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, Hannes Payer, devtools-...@chromium.org, mlippau...@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/objects/slots-inl.h
Line 494, Patchset 14:
void MemsetTagged(Tagged_t* start, Tagged<MaybeObject> value, size_t count) {Leszek Swirski . resolved
nit: this could call the Tagged_t overload now.
Michael Lippautz
Done
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Sep 5, 2025, 9:58:54 AM (2 days ago) Sep 5
to Michael Lippautz, Leszek Swirski, chrom...@appspot.gserviceaccount.com, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
14 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/objects/slots-inl.h
Insertions: 2, Deletions: 3.
@@ -493,12 +493,11 @@
void MemsetTagged(Tagged_t* start, Tagged<MaybeObject> value, size_t count) {
#ifdef V8_COMPRESS_POINTERS
- // CompressAny since many callers pass values which are not valid objects.
Tagged_t raw_value = V8HeapCompressionScheme::CompressAny(value.ptr());
- MemsetUint32(start, raw_value, count);
+ MemsetTagged(start, raw_value, count);
#else
Address raw_value = value.ptr();
- MemsetPointer(start, raw_value, count);
+ MemsetTagged(start, raw_value, count);
#endif
}
```
Change information
Commit message:
[runtime] Optimize JSObject creation from JSON builder
Add a fast path that avoids
- the embedder field check
- the write barrier for the properties in case we set the empty array
Change-Id: I295e016c5cb79ed253a1e9a68c608c2e9d0a1a67
Bug: 442989225
Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Michael Lippautz <mlip...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#102283}
Files:
- M src/api/api-natives.cc
- M src/builtins/builtins-arraybuffer.cc
- M src/builtins/builtins-dataview.cc
- M src/common/globals.h
- M src/debug/debug-wasm-objects.cc
- M src/heap/factory.cc
- M src/heap/factory.h
- M src/json/json-parser.cc
- M src/objects/js-objects-inl.h
- M src/objects/js-objects.h
- M src/objects/slots-inl.h
- M src/objects/slots.h
- M src/utils/memcopy.h
Change size: L
Delta: 13 files changed, 167 insertions(+), 132 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Leszek Swirski
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Sep 6, 2025, 4:01:39 AM (yesterday) Sep 6
to V8 LUCI CQ, Leszek Swirski, chrom...@appspot.gserviceaccount.com, Hannes Payer, devtools-...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Michael Lippautz has created a revert of this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages