Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Float32Array has insufficient precision in call tree timings - incorrect byte sums displayed in call tree #5310

Open
mstange opened this issue Jan 13, 2025 · 1 comment
Labels
call tree Related to the call tree panel

Comments

@mstange
Copy link
Contributor

mstange commented Jan 13, 2025

Profile: https://share.firefox.dev/40hmh0v

In this profile, the call tree displays "Total Size (bytes): 485,408,992" for the root node.
But if you run filteredThread.samples.weight.reduce((a, b) => a + b, 0) on the console, you get a sum of 485,409,290.

Those two values should be the same but they aren't. One ends in 8,992 (incorrect) and the other ends in 9,290 (correct).

This happens because the call tree timings store the total value per node in a Float32Array. But 32-bit floating point values can only exactly represent integers up to 16,777,216.

I must have chosen Float32Array when I wasn't thinking of byte values.

┆Issue is synchronized with this Jira Task

@mstange mstange changed the title Float32Array loses precision in call tree timings - incorrect byte sums displayed in call tree Float32Array has insufficient precision in call tree timings - incorrect byte sums displayed in call tree Jan 13, 2025
@mstange
Copy link
Contributor Author

mstange commented Jan 13, 2025

We can just change this to Float64Array. The only question is whether we should wait until #4900 lands, to avoid conflicts.

@canova canova added the call tree Related to the call tree panel label Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
call tree Related to the call tree panel
Projects
None yet
Development

No branches or pull requests

2 participants