[runtime] Improve derived class name inference [v8/v8 : main]
0 views
Skip to first unread message
Igor Sheludko (Gerrit)
Jun 11, 2026, 11:48:42 AM (3 days ago) Jun 11
to Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz and Patrick Thier
Igor Sheludko added 7 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 19 (Latest):
Igor Sheludko . resolved
Patrick, PTAL @ all
Michael, PTAL @ profiler and compiler
File src/codegen/code-stub-assembler.cc
Line 12085, Patchset 16 (Parent):
TNode<Object> CodeStubAssembler::GetConstructor(TNode<Map> map) {Igor Sheludko . unresolved
There were two unused copies of `GetConstructor` logic in CSA and a third version in `CallOrConstructBuiltinsAssembler::GetCompatibleReceiver()`.
File src/heap/factory.cc
Line 3414, Patchset 16:
}Igor Sheludko . unresolved
This is the bottleneck for setting constructor name for both prototypes created by bootstrapper and lazily created prototype objects for user code.
File src/init/bootstrapper.cc
Line 1407, Patchset 15:
kDontAdapt);Igor Sheludko . unresolved
Pass the hole as a prototype to let it setup everything properly.
The `#ifdef DEBUG` block is 11 years old and is not relevant anymore.
Using the `class_name` here just for clarity. It'll be overwritten during context serialization anyway.
File src/objects/js-function-inl.h
Line 480, Patchset 18 (Parent):
v8_flags.harmony_struct);Igor Sheludko . unresolved
This check was incorrect incomplete, there are other legitimate cases. I updated the comment.
File test/cctest/test-heap-profiler.cc
Line 2533, Patchset 19 (Latest):
CHECK(!i::V8HeapExplorer::GetConstructor(i_isolate, *js_obj5).is_null());Igor Sheludko . unresolved
Now we are able to figure out the constructor function (it's `Object`).
File test/inspector/runtime/evaluate-with-generate-preview-expected.txt
Line 855, Patchset 19 (Latest):
description : ObjectIgor Sheludko . unresolved
`Object.create([1,2])` is an `Object` and not an `Array`.
Related details
Attention is currently required from:
- Michael Lippautz
- Patrick Thier
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)
Jun 11, 2026, 12:46:49 PM (3 days ago) Jun 11
to Igor Sheludko, Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko, Michael Lippautz and Patrick Thier
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job linux-r350-perf/speedometer3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/11761620c90000
Related details
Attention is currently required from:
- Igor Sheludko
- Michael Lippautz
- Patrick Thier
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)
Jun 11, 2026, 12:59:41 PM (3 days ago) Jun 11
to Igor Sheludko, Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko, Michael Lippautz and Patrick Thier
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/speedometer3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/137ee094c90000
chromeperf@appspot.gserviceaccount.com (Gerrit)
Jun 11, 2026, 12:59:48 PM (3 days ago) Jun 11
to Igor Sheludko, Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko, Michael Lippautz and Patrick Thier
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m4-mini-perf/speedometer3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/104f9d2ac90000
chromeperf@appspot.gserviceaccount.com (Gerrit)
Jun 11, 2026, 4:24:35 PM (3 days ago) Jun 11
to Igor Sheludko, Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko, Michael Lippautz and Patrick Thier
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/14b86442c90000
chromeperf@appspot.gserviceaccount.com (Gerrit)
Jun 11, 2026, 4:27:04 PM (3 days ago) Jun 11
to Igor Sheludko, Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko, Michael Lippautz and Patrick Thier
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m4-mini-perf/jetstream3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/12055148c90000
chromeperf@appspot.gserviceaccount.com (Gerrit)
Jun 11, 2026, 4:33:58 PM (3 days ago) Jun 11
to Igor Sheludko, Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko, Michael Lippautz and Patrick Thier
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job linux-r350-perf/jetstream3.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/142f03ecc90000
Patrick Thier (Gerrit)
Jun 12, 2026, 5:22:51 AM (2 days ago) Jun 12
to Igor Sheludko, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko and Michael Lippautz
Patrick Thier voted and added 9 comments![]( "Open in Gerrit")
Votes added by Patrick Thier
| Code-Review | +1 |
9 comments
Patchset-level comments
Patrick Thier . resolved
LGTM
Commit Message
Line 21, Patchset 19 (Latest):
- no the need to deal with JSGlobalObject in order to figure outPatrick Thier . unresolved
```suggestion
- no need to deal with JSGlobalObject in order to figure out
```
File src/codegen/code-stub-assembler.cc
Line 12085, Patchset 16 (Parent):
TNode<Object> CodeStubAssembler::GetConstructor(TNode<Map> map) {Igor Sheludko . resolved
There were two unused copies of `GetConstructor` logic in CSA and a third version in `CallOrConstructBuiltinsAssembler::GetCompatibleReceiver()`.
Patrick Thier
Acknowledged
File src/heap/factory.cc
Line 3401, Patchset 19 (Latest):
// Turn it to prototype mode since the next the caller will do is toPatrick Thier . unresolved
```suggestion
// Turn it to prototype mode since the next caller will
```
This is the bottleneck for setting constructor name for both prototypes created by bootstrapper and lazily created prototype objects for user code.
Patrick Thier
Acknowledged
File src/init/bootstrapper.cc
Line 1407, Patchset 15:
kDontAdapt);Igor Sheludko . unresolved
Pass the hole as a prototype to let it setup everything properly.
The `#ifdef DEBUG` block is 11 years old and is not relevant anymore.
Using the `class_name` here just for clarity. It'll be overwritten during context serialization anyway.
Patrick Thier
Can you please add a comment explaining why the hole is used as prototype here?
File src/objects/js-function-inl.h
Line 480, Patchset 18 (Parent):
v8_flags.harmony_struct);Igor Sheludko . resolved
This check was incorrect incomplete, there are other legitimate cases. I updated the comment.
Patrick Thier
Acknowledged
File src/objects/js-objects.cc
Line 652, Patchset 19 (Latest):
!constructor_name->Equals(ReadOnlyRoots(isolate).Object_string())) {Patrick Thier . unresolved
Do we need to check string equality here?
File test/inspector/runtime/evaluate-with-generate-preview-expected.txt
Line 855, Patchset 19 (Latest):
description : ObjectIgor Sheludko . resolved
`Object.create([1,2])` is an `Object` and not an `Array`.
Patrick Thier
Acknowledged
Related details
Attention is currently required from:
- Igor Sheludko
- 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. |
Igor Sheludko (Gerrit)
Jun 12, 2026, 7:36:13 AM (2 days ago) Jun 12
to Patrick Thier, Michael Lippautz, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz
Igor Sheludko voted and added 5 comments![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Commit-Queue | +1 |
5 comments
Patchset-level comments
Commit Message
Line 21, Patchset 19:
- no the need to deal with JSGlobalObject in order to figure outPatrick Thier . resolved
```suggestion
- no need to deal with JSGlobalObject in order to figure out
```
Igor Sheludko
Done
File src/heap/factory.cc
Line 3401, Patchset 19:
// Turn it to prototype mode since the next the caller will do is toPatrick Thier . resolved
```suggestion
// Turn it to prototype mode since the next caller will
```
Igor Sheludko
Done
File src/init/bootstrapper.cc
Line 1407, Patchset 15:
kDontAdapt);Igor Sheludko . resolved
Patrick ThierPass the hole as a prototype to let it setup everything properly.
The `#ifdef DEBUG` block is 11 years old and is not relevant anymore.
Using the `class_name` here just for clarity. It'll be overwritten during context serialization anyway.
Can you please add a comment explaining why the hole is used as prototype here?
Igor Sheludko
I added the explanatory comment to `CreateFunctionForBuiltinWithPrototype` which is used in a couple of other places in this file.
File src/objects/js-objects.cc
Line 652, Patchset 19:
!constructor_name->Equals(ReadOnlyRoots(isolate).Object_string())) {Patrick Thier . resolved
Do we need to check string equality here?
Igor Sheludko
Yes. Empty name and "Object" name are considered too generic, so we want to query `Symbol.toStringTag` property in a hope to find a nicer name.
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. |
Michael Lippautz (Gerrit)
Jun 12, 2026, 7:39:44 AM (2 days ago) Jun 12
to Igor Sheludko, Patrick Thier, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Igor Sheludko
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Igor Sheludko
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. |
Igor Sheludko (Gerrit)
Jun 12, 2026, 9:23:50 AM (2 days ago) Jun 12
to Michael Lippautz, Patrick Thier, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Igor Sheludko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Related details
Attention set is empty
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. |
Igor Sheludko (Gerrit)
Jun 12, 2026, 9:28:02 AM (2 days ago) Jun 12
to Michael Lippautz, Patrick Thier, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, v8-s...@luci-project-accounts.iam.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Igor Sheludko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Commit-Queue | +2 |
1 comment
File test/cctest/test-heap-profiler.cc
Line 2533, Patchset 19:
CHECK(!i::V8HeapExplorer::GetConstructor(i_isolate, *js_obj5).is_null());Igor Sheludko . resolved
Now we are able to figure out the constructor function (it's `Object`).
Igor Sheludko
Acknowledged
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)
Jun 12, 2026, 10:54:52 AM (2 days ago) Jun 12
to Igor Sheludko, Michael Lippautz, Patrick Thier, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@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
22 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/diagnostics/objects-debug.cc
Insertions: 0, Deletions: 2.
@@ -863,7 +863,6 @@
CHECK(IsJSFunction(tuple->value1()));
CHECK(IsString(tuple->value2()));
} else {
- CHECK(new_target_is_base());
CHECK(IsJSFunction(maybe_constructor) ||
IsFunctionTemplateInfo(maybe_constructor) ||
IsString(maybe_constructor) ||
@@ -872,7 +871,6 @@
Context::EMPTY_FUNCTION_INDEX)));
CHECK_IMPLIES(IsString(maybe_constructor), is_prototype_map());
}
- CHECK_IMPLIES(!new_target_is_base(), IsTuple2(maybe_constructor));
}
}
```
Change information
Commit message:
[runtime] Improve derived class name inference
... by recording the value of new.target's function name along side the
constructor value in a Tuple2 stored in initial map's constructor field.
This simplifies the constructor name deduction logic which
a) used to query obj.prototype.constructor's name in attempt to figure
out the name of the derived class,
b) did not respect cross-origin boundaries when it did (a).
Other benefits:
- this also allows to free Map::new_target_is_base bit because the
derived case can be detected simply by checking
IsTuple2(map->GetConstructorRaw()).
- no need to deal with JSGlobalObject in order to figure out
JSGlobalProxy's class name - the class name is now copied from
global object's constructor template to global proxy's
constructor template.
Drive-by: report Proxy objects as "Object" or "Function" to make them
indistinguishable from regular Objects/Functions in stack traces.
NO_IFTTT=Introducing IFTTT for Map::GetConstructor() implementation.
Bug: 522127452
Fixed: 513302107
Fixed: 400895801
Change-Id: I38abf7886b0e13cf0a8cae414131b0c872d648ea
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Commit-Queue: Igor Sheludko <ish...@chromium.org>
Reviewed-by: Patrick Thier <pth...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107954}
Files:
- M src/api/api.cc
- M src/builtins/base.tq
- M src/builtins/builtins-api.cc
- M src/builtins/builtins-call-gen.cc
- M src/builtins/builtins-constructor-gen.cc
- M src/builtins/cast.tq
- M src/codegen/code-stub-assembler.cc
- M src/codegen/code-stub-assembler.h
- M src/compiler/heap-refs.cc
- M src/diagnostics/objects-debug.cc
- M src/diagnostics/objects-printer.cc
- M src/heap/factory.cc
- M src/heap/factory.h
- M src/init/bootstrapper.cc
- M src/objects/js-function-inl.h
- M src/objects/js-function.cc
- M src/objects/js-function.h
- M src/objects/js-objects.cc
- M src/objects/js-objects.h
- M src/objects/js-objects.tq
- M src/objects/map-inl.h
- M src/objects/map.cc
- M src/objects/map.h
- M src/objects/map.tq
- M src/objects/templates.cc
- M src/profiler/heap-snapshot-generator.cc
- M test/cctest/test-api.cc
- M test/cctest/test-heap-profiler.cc
- M test/cctest/test-serialize.cc
- M test/inspector/regress/regress-crbug-980018-expected.txt
- M test/inspector/runtime/evaluate-with-generate-preview-expected.txt
- A test/mjsunit/regress/regress-513302107.js
Change size: XL
Delta: 32 files changed, 741 insertions(+), 306 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Patrick Thier, +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Igor Sheludko (Gerrit)
Jun 13, 2026, 4:52:14 PM (16 hours ago) Jun 13
to v8-s...@luci-project-accounts.iam.gserviceaccount.com, Michael Lippautz, Patrick Thier, Hannes Payer, android-bu...@system.gserviceaccount.com, chrom...@appspot.gserviceaccount.com, devtools-...@chromium.org, mlippau...@chromium.org, dmercadi...@chromium.org, v8-re...@googlegroups.com
Igor Sheludko has created a revert of this change![]( "Open in Gerrit")
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