Use base 10 when calculating memory#5810
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5810 +/- ##
=======================================
Coverage 85.57% 85.57%
=======================================
Files 320 320
Lines 31448 31448
Branches 8570 8656 +86
=======================================
Hits 26911 26911
Misses 4106 4106
Partials 431 431 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey @fatadel, thanks for the PR! The example profile you shared is a regular sampling profile. Could you capture and share a memory profile using for example the "Native Allocation" feature in the about:profiling? |
|
Thanks for the hint, @canova! Updated the links. |
canova
left a comment
There was a problem hiding this comment.
Looks good to me, thanks!
Btw, I updated the title to fix the typo and also changed the capitalization.
src/test/unit/format-numbers.test.ts
Outdated
|
|
||
| it('can return values with kilobyte precision', () => { | ||
| expect(formatGigaBytes(1234567890, 3, 2, 1024)).toBe('1GB 153MB 385KB'); | ||
| expect(formatGigaBytes(1234567890, 3, 2, 1024)).toBe('1GB 234MB 568KB'); |
There was a problem hiding this comment.
| expect(formatGigaBytes(1234567890, 3, 2, 1024)).toBe('1GB 234MB 568KB'); | |
| expect(formatGigaBytes(1234567890, 3, 2, 1000)).toBe('1GB 234MB 568KB'); |
nit: it would be good to change these precision values to 10 based too. (here and below)
There was a problem hiding this comment.
Thanks for your review and for the nit, @canova! I've incorporated it, will merge now.
Closes #5485.
I hope I correctly understood the issue and the following intentions. Also, there were 2 different opinions when we were discussing the issue:
I agree with opinion 1, as it makes things easier and consistent imo. But feel to propose other solutions.
Example profile: before / after.