Skip to content

Show the precise CPU usage information in the tooltip instead of the smoothed values#4623

Merged
canova merged 1 commit into
firefox-devtools:mainfrom
canova:activity-graph-precise-cpu
Jun 7, 2023
Merged

Show the precise CPU usage information in the tooltip instead of the smoothed values#4623
canova merged 1 commit into
firefox-devtools:mainfrom
canova:activity-graph-precise-cpu

Conversation

@canova

@canova canova commented May 17, 2023

Copy link
Copy Markdown
Member

Previously we were using the smoothed values for the CPU information which wasn't correct. With this PR, we are now keeping the raw calculated CPU usage values per pixel, and then get these numbers while we are doing the hittesting to show them in the tooltip.

Example profile: production / deploy preview

@canova canova requested a review from julienw May 17, 2023 13:59
@codecov

codecov Bot commented May 17, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (3e23f91) 88.47% compared to head (c98de7e) 88.47%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4623   +/-   ##
=======================================
  Coverage   88.47%   88.47%           
=======================================
  Files         295      295           
  Lines       26239    26241    +2     
  Branches     7072     7072           
=======================================
+ Hits        23216    23218    +2     
  Misses       2812     2812           
  Partials      211      211           
Impacted Files Coverage Δ
src/components/shared/thread/ActivityGraphFills.js 96.49% <100.00%> (+0.02%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@julienw julienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is much better!

Comment thread src/test/components/SampleTooltipContents.test.js Outdated
it('renders the CPU usage properly in the areas where blur is applied', () => {
const profile = getProfileWithCPU([null, 400, 580, 1000], 'µs');

// Let's check the second threadCPUDelta value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the one with 580, right? And the result us 58% because the max is 1000, is that right? Can you please write a small comment explaining more why the expected result is this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct. Added a comment.

@@ -53,9 +53,22 @@ function getSamplesPixelPosition(
// +--------------------+--------------------+--------------------+

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please update the comment here :-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thanks!

@canova canova force-pushed the activity-graph-precise-cpu branch from 0989d88 to c98de7e Compare June 7, 2023 11:40
@canova canova merged commit feed2ba into firefox-devtools:main Jun 7, 2023
@canova canova deleted the activity-graph-precise-cpu branch June 7, 2023 12:01
@canova canova mentioned this pull request Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants