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
[Profiling] Adding normalize by time and scale factor on Diff TopN functions page #159394
[Profiling] Adding normalize by time and scale factor on Diff TopN functions page #159394
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
exclusiveCPU: | ||
exclusiveCPU - | ||
(comparisonRow.CountExclusive / comparisonTopNFunctions.TotalCount) * 100, | ||
inclusiveCPU: | ||
inclusiveCPU - | ||
(comparisonRow.CountInclusive / comparisonTopNFunctions.TotalCount) * 100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: I'm not sure if I should scale the CPU values too! @elastic/profiling-ui does anybody know it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that scaling the CPU values would require scaling both the numerator and denominator, which would cancel out the scaling factor when dividing. So I don't believe it's necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To show my work and prove it to myself also, here's my reasoning:
- All definitions apply equally to the inclusive and exclusive CPU values.
- We'll assume inclusive without loss of generality for any CPUn and Totaln.
- CPU1 is the baseline CPU
- CPU2 is the comparison CPU
- Total1 is the baseline total count
- Total2 is the comparison total count
- SF1 is the baseline scaling factor
- SF2 is the comparison scaling factor
Without scaling
CPUdiff = 100 * (CPU1 / Total1) - 100 * (CPU2 / Total2)
or
CPUdiff = 100 * [(CPU1 / Total1) - (CPU2 / Total2)]
With scaling
CPUdiff = 100 * [(SF1* CPU1) / (SF1 * Total1)] - 100 * [(SF2 * CPU2) / (SF2 * Total2)]
or this after cancellation:
CPUdiff = 100 * (CPU1 / Total1) - 100 * (CPU2 / Total2)
which is the same as without scaling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL
wow, great explanation! thank you! 🤓
…-ref HEAD~1..HEAD --fix'
…condes/kibana into profiling-functions-normalize-by
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I answered your question about whether we need to scale the CPU values.
exclusiveCPU: | ||
exclusiveCPU - | ||
(comparisonRow.CountExclusive / comparisonTopNFunctions.TotalCount) * 100, | ||
inclusiveCPU: | ||
inclusiveCPU - | ||
(comparisonRow.CountInclusive / comparisonTopNFunctions.TotalCount) * 100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that scaling the CPU values would require scaling both the numerator and denominator, which would cancel out the scaling factor when dividing. So I don't believe it's necessary.
exclusiveCPU: | ||
exclusiveCPU - | ||
(comparisonRow.CountExclusive / comparisonTopNFunctions.TotalCount) * 100, | ||
inclusiveCPU: | ||
inclusiveCPU - | ||
(comparisonRow.CountInclusive / comparisonTopNFunctions.TotalCount) * 100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To show my work and prove it to myself also, here's my reasoning:
- All definitions apply equally to the inclusive and exclusive CPU values.
- We'll assume inclusive without loss of generality for any CPUn and Totaln.
- CPU1 is the baseline CPU
- CPU2 is the comparison CPU
- Total1 is the baseline total count
- Total2 is the comparison total count
- SF1 is the baseline scaling factor
- SF2 is the comparison scaling factor
Without scaling
CPUdiff = 100 * (CPU1 / Total1) - 100 * (CPU2 / Total2)
or
CPUdiff = 100 * [(CPU1 / Total1) - (CPU2 / Total2)]
With scaling
CPUdiff = 100 * [(SF1* CPU1) / (SF1 * Total1)] - 100 * [(SF2 * CPU2) / (SF2 * Total2)]
or this after cancellation:
CPUdiff = 100 * (CPU1 / Total1) - 100 * (CPU2 / Total2)
which is the same as without scaling.
Relevant changes: