Repository navigation
Conversation
This bumps the profile version to add the new column. This addresses the case where two different JS bundles have an "original source" with the same path in their source maps. E.g. both `https://foo.com/bundle.js` and `https://bar.com/bundle.js` might contain a source with the path `webpack://src/index.js`. In theory, source map resolution could just create two separate sources for the two original sources with the colliding paths, and not write down in the profile which bundle they came from. That would work, but it would not allow correct profile merging in the compare view; the merged profile might accidentally merge sources that shouldn't be merged. Remembering the bundle URL additionally lets us do the following (though none of those things are attempted in this stack): - For profiles where the source content has been sanitized out, we could have a feature in profiler-edit to put the source content back in. - Relative path resolution: When displaying original sources with relative paths, we could display a resolved absolute path (resolved relative to the bundle) in the UI. - We could display the bundle URL in any profiler-cli output that deals with sources. For the upgrader, I considered making it so that it recovers the generated source via funcTable / frameTable / sourceLocationTable, and splits sources (+ makes corresponding source locations) for profiles where collisions already happened, but it was too complicated. If we want the invariant that sourceTable.generatedSource always matches funcTable.source, then we will need to add this more complicated upgrader in a future version.
Fixes firefox-devtools#6423. Original sources from source maps were identified by their URL alone. Two bundles can list the same URL for different files, for example two Speedometer suites that each bundle their own version of ../../node_modules/react-dom/cjs/react-dom-client.production.js. Symbolication then treated them as one file: function names were looked up in whichever file's content was seen first, and both bundles' functions shared one source table row. Now an original source is identified by the bundle whose source map lists it, plus the URL as written in that source map. The worker response carries a list of original sources, and resolutions refer to entries by index. New source rows record their bundle in generatedSource. The apply step no longer tries to reuse any existing original sources in the source table of the profile. Instead, it adds a row for an original source when the first func or frame that refers to it is applied. If a bundler produced multiple "bundle chunks" which both include the same original source, we will now have two different source table entries for them. I think that's ok; I can't imagine this happens all that commonly.
Profile merging identified sources without an id by their filename. It now identifies original sources from source maps by their bundle and filename, like source map symbolication does. Native symbolication looks up its file sources by filename among the sources without an id. It now skips original sources from source maps, since their URLs are only unique per bundle.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6432 +/- ##
==========================================
+ Coverage 84.78% 84.81% +0.02%
==========================================
Files 358 358
Lines 38946 38981 +35
Branches 10917 11041 +124
==========================================
+ Hits 33022 33061 +39
+ Misses 5488 5483 -5
- Partials 436 437 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Main | Deploy preview
Fixes #6423.
Profile recorded with fixed version (requires deploy preview due to version bump): https://share.firefox.dev/4BvwYhP