Display size and count of blink dom objects in memory-infra. [v8/v8 : main]
鹏强王 (Gerrit)
鹏强王 added 2 comments![]( "Open in Gerrit")
Display size and count of blink dom objects in memory-infra.鹏强王Please add a title to your CL and format it accordingly.
"""
TitleDescription...
Bug:
"""
Done
* Statistics for object allocated on the page. The dom objects name was鹏强王We generally don't refer to terms like DOM from V8 as that concept doesn't exist here on the API.
If we go through with this change, this would need to saysomething along the lnes of
"Statistics for object allocated on the page. Note that objects without names are coerced to a single type."
Done
Related details
- Michael Lippautz
- Omer Katz
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Omer Katz added 1 comment![]( "Open in Gerrit")
鹏强王Names are intentionally omitted in production builds (unless explicitly enabled) to avoid increasing the binary size.
Always having the names available would indeed be convenient for debugging, but it's not currently an option.
You could try using "Chrome for Testing" instead (see https://developer.chrome.com/blog/chrome-for-testing/), which I believe should have the names enabled.
Omer KatzI approve of your opinion that name omitted in production builds to avoid increasing the binary size. But some blink objects has explicitly specified names inherited NameProvider, we can obtain these blink objects name using NameProvide::GetHumanReadableName, the Human-readable name of these object is used to show in devtools heap snapshot. it doesn't increase the binary size. I think it is necessary that show these blink object in memory-infra.
Thanks for clarifying. I misunderstood your intention.
Related details
- Michael Lippautz
- 鹏强王
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Omer Katz added 3 comments![]( "Open in Gerrit")
Code looks good to me. Just the comments can be slightly improved. Thanks.
/**
* Statistics for object allocated on the page. If an object has explicit
* names by inherited from NameProvider, its name will display in
* memory-infra, and other objects without name are coerced to a
* single type unless enable CPPGC_SUPPORTS_OBJECT_NAMES.
*/
std::vector<ObjectStatsEntry> object_statistics;Please replace this comment with the following:
```
/**
* Statistics for object allocated on the page. If an object provides a
* name by inheriting from NameProvider, its name will be displayed in
* memory-infra. Other objects, without an explicit name, are merged under a
* single type unless the CPPGC_SUPPORTS_OBJECT_NAME build flag is enabled.
*/
```
// It records the internal cpp object with "InternalNode", if you want to
// display more detailed type name of internal cpp object, please enable
// CPPGC_SUPPORTS_OBJECT_NAMES macro.
const auto it = type_map.insert({header->GetName().value, type_map.size()});Please replace this paragraph with the following:
```
// Internal cpp objects that do not inherit from NameProvider are recorded
// as and merged under "InternalNode". For more detailed statistics Enable
// the build flag CPPGC_SUPPORTS_OBJECT_NAMES to instead use the C++ class
// name for more detailed statistics.
```
Related details
- Michael Lippautz
- 鹏强王
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
鹏强王 (Gerrit)
鹏强王 added 3 comments![]( "Open in Gerrit")
Code looks good to me. Just the comments can be slightly improved. Thanks.
Thank you for your advice. I have improved it.
* Statistics for object allocated on the page. If an object has explicit
* names by inherited from NameProvider, its name will display in
* memory-infra, and other objects without name are coerced to a
* single type unless enable CPPGC_SUPPORTS_OBJECT_NAMES.
*/
std::vector<ObjectStatsEntry> object_statistics;Please replace this comment with the following:
```
/**
* Statistics for object allocated on the page. If an object provides a
* name by inheriting from NameProvider, its name will be displayed in
* memory-infra. Other objects, without an explicit name, are merged under a
* single type unless the CPPGC_SUPPORTS_OBJECT_NAME build flag is enabled.
*/
```
Done
// It records the internal cpp object with "InternalNode", if you want to
// display more detailed type name of internal cpp object, please enable
// CPPGC_SUPPORTS_OBJECT_NAMES macro.
const auto it = type_map.insert({header->GetName().value, type_map.size()});Please replace this paragraph with the following:
```
// Internal cpp objects that do not inherit from NameProvider are recorded
// as and merged under "InternalNode". For more detailed statistics Enable
// the build flag CPPGC_SUPPORTS_OBJECT_NAMES to instead use the C++ class
// name for more detailed statistics.
```
- Michael Lippautz
- Omer Katz
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Omer Katz voted and added 1 comment![]( "Open in Gerrit")
- Michael Lippautz
- 鹏强王
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
鹏强王 (Gerrit)
鹏强王 added 1 comment![]( "Open in Gerrit")
* Statistics for object allocated on the page. The dom objects name was鹏强王We generally don't refer to terms like DOM from V8 as that concept doesn't exist here on the API.
If we go through with this change, this would need to saysomething along the lnes of
"Statistics for object allocated on the page. Note that objects without names are coerced to a single type."
Done
- Anton Bikineev
- Michael Lippautz
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Michael Lippautz added 3 comments![]( "Open in Gerrit")
Please avoid adding amore reviewers when not necessary.
Kevin Wang <wangpe...@bytedance.com>Please sort your name in accordingly.
* memory-infra. Other objects, without an explicit name, are merged under aAgain, the term `memory-infra` is not known to V8 or the API here. Memory infra may change (and actually does change) over time and this comment is just designed to be outdated.
Related details
- Anton Bikineev
- 鹏强王
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
鹏强王 (Gerrit)
鹏强王 added 2 comments![]( "Open in Gerrit")
Please sort your name in accordingly.
Done
* memory-infra. Other objects, without an explicit name, are merged under aAgain, the term `memory-infra` is not known to V8 or the API here. Memory infra may change (and actually does change) over time and this comment is just designed to be outdated.
- Michael Lippautz
- Omer Katz
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Omer Katz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Omer Katz
| Code-Review | +1 |
- Michael Lippautz
- 鹏强王
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
鹏强王 (Gerrit)
鹏强王 added 1 comment![]( "Open in Gerrit")
鹏强王Please avoid adding amore reviewers when not necessary.
Can you help to review my code? I have already modify comment and author with your opinion. Thanks
Related details
- Michael Lippautz
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Lippautz
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Related details
- 鹏强王
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Display size and count of blink dom objects in memory-infra.
The blink contains two type of object: the dom objects have explicitly
specified names via using NameProvider, and other cpp internal objects
have hide names 'InternalNode'. Now we must enable
CPPGC_SUPPORTS_OBJECT_NAMES macro to show the name and size of all blink
objects in memory-infra. I think it is better that display the count and
size of these dom objects inherited from NameProvider without enabling
CPPGC_SUPPORTS_OBJECT_NAMES macro in release version.
It will provide great convenience to help us debug the memory issues of
the blink object.
- M AUTHORS
- M include/cppgc/heap-statistics.h
- M src/heap/cppgc/heap-statistics-collector.cc
Code-Review: +1 by Omer Katz, +1 by Michael Lippautz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |