Skip to content

Conversation

@mstange
Copy link
Contributor

@mstange mstange commented Jan 30, 2025

Production | Deploy preview - Double-click the first row in the call tree and then click the asm button

Screenshot 2026-01-21 at 10 46 59 PM

If a JS function is JIT compiled multiple times, then it's currently impossible to see the assembly code for each compilation. If you double-click the call node for the function, the assembly view shows an arbitrary compilation. It doesn't even try to find the one with the highest sample count. The same is true for functions that were inlined into multiple different functions.

This PR allows navigating between the different compilations.

I'll make a separate PR to pick the native symbol with the highest sample count.

@mstange mstange force-pushed the assembly-view-prev-next branch from 49d95a4 to 139d000 Compare January 31, 2025 05:06
@mstange mstange force-pushed the assembly-view-prev-next branch from 139d000 to 74b06df Compare February 14, 2025 04:16
@mstange mstange force-pushed the assembly-view-prev-next branch from 74b06df to 39b83d2 Compare February 24, 2025 00:23
@mstange mstange force-pushed the assembly-view-prev-next branch from 39b83d2 to 7ea21d4 Compare July 10, 2025 17:55
@mstange mstange force-pushed the assembly-view-prev-next branch from 7ea21d4 to 4112ac4 Compare January 8, 2026 09:12
@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.75%. Comparing base (ac4bb14) to head (c3eaddf).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...mponents/app/AssemblyViewNativeSymbolNavigator.tsx 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5349      +/-   ##
==========================================
+ Coverage   85.71%   85.75%   +0.04%     
==========================================
  Files         317      318       +1     
  Lines       31216    31240      +24     
  Branches     8499     8600     +101     
==========================================
+ Hits        26756    26790      +34     
+ Misses       4030     4020      -10     
  Partials      430      430              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mstange mstange force-pushed the assembly-view-prev-next branch 2 times, most recently from f88be1c to 8ecd06e Compare January 14, 2026 20:37
mstange added a commit that referenced this pull request Jan 19, 2026
A small behavior-neutral change that I've extracted from #5349.
mstange added a commit that referenced this pull request Jan 19, 2026
…iew (#5759)

[Production](https://share.firefox.dev/4qXrKoY) | [Deploy
preview](https://deploy-preview-5759--perf-html.netlify.app/public/f2eatktw200exzsbzps5xg0gpjkb785cjj1yz8r/calltree/?globalTrackOrder=a0978465312&hiddenGlobalTracks=0w8a&hiddenLocalTracksByPid=46164-0w3~46165-0~46175-0~46171-0~46589-0~46180-0~46588-0~46174-0~46168-01~46177-0&range=m29020&thread=e&transforms=ff-1037~ff-1046&v=12)

Fixes #5758 and cleans things up a bit.

Having the samples passed to `getBottomBoxInfoForCallNode` will also
make it easier to address the TODO about picking the native symbol with
the highest sample count, but I'll do that in a separate PR (or probably
as part of #5349).
@mstange mstange force-pushed the assembly-view-prev-next branch from 8ecd06e to 1a8f46a Compare January 22, 2026 03:41
@mstange mstange marked this pull request as ready for review January 22, 2026 03:42
@mstange mstange requested a review from a team as a code owner January 22, 2026 03:42
@mstange mstange requested a review from canova January 22, 2026 03:42
@mstange mstange self-assigned this Jan 22, 2026
@mstange
Copy link
Contributor Author

mstange commented Jan 22, 2026

This is now ready for review.

@mstange mstange changed the title [Deploy preview] Allow seeing different assembly code for the same function Allow seeing different assembly code for the same function Jan 22, 2026
Copy link
Member

@canova canova left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me, but can we also make the x of y text in the UI localizable?

return (
<>
<h3 className="bottom-box-title-trailer">
{index !== null && count > 1 ? `${index + 1} of ${count}` : ''}
Copy link
Member

Choose a reason for hiding this comment

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

Can we also make the x of y string localizable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right! Added a commit for that.

return (
<>
<h3 className="bottom-box-title-trailer">
{index !== null && count > 1 ? `${index + 1} of ${count}` : ''}
Copy link
Member

Choose a reason for hiding this comment

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

Also, index !== null shouldn't be reachable now thanks to the early return above. We can remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, fixed.

Copy link
Contributor

@flodolo flodolo left a comment

Choose a reason for hiding this comment

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

Nazım is correct, we shouldn't have hardcoded English text.

@mstange mstange force-pushed the assembly-view-prev-next branch from 1a8f46a to 4a535bc Compare January 23, 2026 15:41
@mstange mstange force-pushed the assembly-view-prev-next branch from 4a535bc to 65dfc52 Compare January 23, 2026 15:42
@mstange mstange requested a review from flodolo January 23, 2026 15:43
Copy link
Member

@canova canova left a comment

Choose a reason for hiding this comment

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

Thanks!

@mstange mstange force-pushed the assembly-view-prev-next branch from 65dfc52 to c3eaddf Compare January 23, 2026 16:21
@mstange mstange enabled auto-merge January 23, 2026 16:22
@mstange mstange merged commit e927183 into firefox-devtools:main Jan 23, 2026
20 checks passed
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.

3 participants