Skip to content

Make mergeFunction fast#4895

Merged
mstange merged 1 commit into
firefox-devtools:mainfrom
mstange:fast-merge
Jan 23, 2024
Merged

Make mergeFunction fast#4895
mstange merged 1 commit into
firefox-devtools:mainfrom
mstange:fast-merge

Conversation

@mstange

@mstange mstange commented Jan 22, 2024

Copy link
Copy Markdown
Contributor

Merging a function on this profile takes about 2.5 seconds on my machine at the moment. This patch reduces that time to just under 1 second, and that second is spent almost entirely in getCallNodeInfo.

Before: https://share.firefox.dev/3SbzpQy (1422 samples in mergeFunction)
After: https://share.firefox.dev/3tVp3fQ (46 samples in mergeFunction, 30x faster)

@mstange mstange requested a review from julienw January 22, 2024 21:11
@mstange mstange self-assigned this Jan 22, 2024
@codecov

codecov Bot commented Jan 22, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7afd2d5) 88.41% compared to head (21b4348) 88.41%.

❗ Current head 21b4348 differs from pull request most recent head 48e87ca. Consider uploading reports for the commit 48e87ca to get more accurate results

Files Patch % Lines
src/profile-logic/transforms.js 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4895      +/-   ##
==========================================
- Coverage   88.41%   88.41%   -0.01%     
==========================================
  Files         303      303              
  Lines       27134    27134              
  Branches     7332     7332              
==========================================
- Hits        23991    23990       -1     
- Misses       2925     2926       +1     
  Partials      218      218              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@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.

r+ for the changes to the mergeFunction transform that look great to me, however the part in ActivityGraphFills looks like a leftover from another work, unless I'm missing something. Can you please revert that part?
thanks!

Comment thread src/components/shared/thread/ActivityGraphFills.js Outdated
Comment thread src/components/shared/thread/ActivityGraphFills.js Outdated
Comment thread src/components/shared/thread/ActivityGraphFills.js
Before: https://share.firefox.dev/3SbzpQy (1422 samples in mergeFunction)
After: https://share.firefox.dev/3tVp3fQ (46 samples in mergeFunction, 30x faster)
@mstange

mstange commented Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

I've reverted the ActivityGraphFills.js part.

@mstange mstange merged commit 5ab5512 into firefox-devtools:main Jan 23, 2024
@julienw julienw mentioned this pull request Feb 6, 2024
julienw added a commit that referenced this pull request Feb 6, 2024
[@canova Nazım Can Altınova] Return null instead of throwing an error if it fails to get the resource name in active tab view (#4905)
[@fqueze Florian Quèze] Improve the precision of the getFittedText implementation. (#4893)
[@fqueze Florian Quèze] Show marker count next to the marker name in the marker chart when a marker is hovered. (#4892)
[@mstange Markus Stange] Convert some code to use the non-inverted call node table even while we're showing the inverted call tree (Merge PR #4899)
[@canova Nazım Can Altınova] Bring the max stacking depth limit back for marker timings (#4898)
[@mstange Markus Stange] Various preparations in call tree code for fast inverted tree (Merge PR #4897)
[@mstange Markus Stange] Move call node path/index conversion functions into CallNodeInfo (Merge PR #4896)
[@mstange Markus Stange] Make mergeFunction fast (Merge PR #4895)
[@gthb Gunnlaugur Thor Briem] feat: support from-url profiles in compare (#4875)

And all the locale contributors:
de: Michael Köhler
el: Jim Spentzos
en-GB: Ian Neal
es-CL: ravmn
fr: Théo Chevalier
fy-NL: Fjoerfoks
ia: Melo46
it: Francesco Lodolo [:flod]
nl: Fjoerfoks, Mark Heijl
pt-BR: Marcelo Ghelman
ru: Valery Ledovskoy
sv-SE: Andreas Pettersson
uk: Artem Polivanchuk
zh-CN: 你我皆凡人, Olvcpr423, Pontoon
zh-TW: Pin-guang Chen
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