v8: avoid truncating cppgc heap statistics - #64696
Conversation
nodejs-github-bot
added
c++
Convert cppgc size_t counters directly to JavaScript Numbers instead of narrowing them to uint32_t. This prevents v8.getCppHeapStatistics() values from wrapping when a cppgc heap statistic exceeds 4 GiB. Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64696 +/- ##
==========================================
- Coverage 90.14% 90.13% -0.01%
==========================================
Files 741 741
Lines 242112 242107 -5
Branches 45605 45604 -1
==========================================
- Hits 218244 218224 -20
- Misses 15360 15379 +19
+ Partials 8508 8504 -4
🚀 New features to boost your workflow:
|
| Number::New(isolate, | ||
| static_cast<double>(object_stats.allocated_bytes)), | ||
| Number::New(isolate, | ||
| static_cast<double>(object_stats.object_count))}; |
There was a problem hiding this comment.
I wonder if we should instead optionally be handling these as bigint/uint64_t. Accept the truncation by default, add an option to return bigint instead...
v8.getCppHeapStatistics({ detailLevel: '...', bigint: true });
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
I wonder if we should instead optionally be handling these as bigint/uint64_t. Accept the truncation by default, add an option to return bigint instead...
Are any of these realistically in danger of hitting kMaxSafeInteger?
This comment was marked as spam.
This comment was marked as spam.
This comment has been minimized.
This comment has been minimized.
Renegade334
left a comment
Renegade334
left a comment
There was a problem hiding this comment.
This seems like a pure improvement to me, the likelihood of any of these values being exabytes in size seems exceedingly remote, whereas wrapping after a few gigabytes seems like a genuine pitfall.
This comment has been minimized.
This comment has been minimized.
This comment was marked as spam.
This comment was marked as spam.
5 participants
Convert cppgc size_t counters directly to JavaScript Numbers instead of narrowing them to uint32_t.
This prevents v8.getCppHeapStatistics() values from wrapping when a cppgc heap statistic exceeds 4 GiB.