Debugger freezes when viewing very long single-line JS file (3MB)
Categories
(DevTools :: Debugger, defect, P2)
Tracking
(firefox-esr91 unaffected, firefox-esr102 verified, firefox102 wontfix, firefox103 wontfix, firefox104 verified, firefox105 verified)
People
(Reporter: danweiss, Assigned: nchevobbe)
References
(Blocks 2 open bugs, Regression)
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr102+
|
Details |
(deleted),
image/gif
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0
Steps to reproduce:
These are steps I found that can trigger the bug:
Have your browser window be at least 1224 pixels wide
Open a stream on twitch.tv where there is video playing and chatting
Press F12 to open developer tools
Go to Inspector.
In search HTML, look for "showmore"
Should see <button class="ScCoreLink-sc-udwpw5-0 gBLUEB tw-link"...
, with an "event" button.
Click on the "event" button
For the "onClick" row, click on the 'View In Debugger' button (it's a curved arrow pointing to two lines, it's to the left of the words "react" "bubbling")
It has switched over to the Debugger, now you are looking at a single line of javascript that is over 3MB large, and this is badly bogging down Firefox.
This seems to be a regression, as Firefox ESR 91 does not have this problem. I suspect this may have to do with the process isolation feature introduced around Firefox 100.
Actual results:
Firefox's "Javascript Debugger" in Web Developer Tools badly bogs down on a 3MB single line javascript file, taking over 1 minute before it resumes. Worse if there is other things happening in the background, such as video or chat.
Expected results:
The Debugger UI should have remained responsive and not froze.
Comment 1•2 years ago
|
||
The Bugbug bot thinks this bug should belong to the 'DevTools::General' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
Assignee | ||
Updated•2 years ago
|
For reference, this is the particular .js file that I encountered the issue on: https://static.twitchcdn.net/assets/core-58284c7f5f945e992e20.js . The file is 2,969,556 bytes large, and contains some Unicode characters.
In case that file goes missing, I made an archival copy at https://www.dwedit.org/files/.bugzilla/core-58284c7f5f945e992e20.7z (7-zip compressed)
When I view the .js file in the browser directly then use "View Source" view-source:https://static.twitchcdn.net/assets/core-58284c7f5f945e992e20.js
, I also see a huge single line. If you scroll, Firefox shows blank space, then 2 seconds later, the text appears. However, the browser does not freeze when waiting for the text to appear, it remains responsive. As a side note, text at the end of the file has incorrect character spacing and looks weird, but that's a different bug.
Comment 3•2 years ago
|
||
Sounds similar to Bug 1778758. Surprising to see that it is a regression though. Will run mozregression.
Comment 4•2 years ago
|
||
As far as I can tell, this regressed in the following push: https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=6f0a8dddad511ef68fae3a4ab07cc9336516cc3a
(that push was probably wrong, so for the record, mozregression stopped at https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=73b74d9be741b4bc64964d2cf8a72a995badfb09&tochange=63ef5a7d2b1051617b4549384022cd9889d512da for me)
One notable difference is that before the content was displayed without any syntax highlighting, but relatively quickly. Now we seem to have syntax highlighting immediately, but the browser freezes.
Comment 5•2 years ago
|
||
Actually a regression from Bug 1754407, specifically from the last patch in the queue: https://hg.mozilla.org/integration/autoland/rev/dc88906824026db14115269cdc9f35f0ed3edbc6
Updated•2 years ago
|
Comment 7•2 years ago
|
||
From the other bug: profile at https://share.firefox.dev/3atrL2o
We should bail out of highlighting for large files, there seems to be a mechanism in CodeMirror to do that: https://searchfox.org/mozilla-central/rev/a352cc827575823676717d53766c39710b54201a/devtools/client/shared/sourceeditor/editor.js#161 we should check if this works
Updated•2 years ago
|
Comment 9•2 years ago
|
||
Not much progress yet, but I can confirm the main difference between before and after the regressing changeset is that before we never used to set the editor mode to "javascript" for those long source files, whereas we do now.
Assignee | ||
Comment 10•2 years ago
|
||
The maxHighlightLength
seems to be working as intended: if I evaluate the following in the console eval(("x= 2;".repeat(210)) + "debugger;" );
it pauses the debugger, and I can see that the debugger
statement isn't highlighted.
Comment 11•2 years ago
|
||
:ochameau, since you are the author of the regressor, bug 1754407, could you take a look?
For more information, please visit auto_nag documentation.
Comment 12•2 years ago
|
||
Will investigate today, we could force the mode to text for very big files (which seemed to be the behavior earlier)
Comment 13•2 years ago
|
||
Hm, it seems like that's not an ideal fix -- I know that the Chromium version of the debugger (which, I understand is not the same; but I think it still uses CodeMirror) doesn't choke on syntax highlighting like the current version of the FF debugger.
Comment 14•2 years ago
|
||
(In reply to Brooke Baer [:brookerose1312] from comment #13)
Hm, it seems like that's not an ideal fix -- I know that the Chromium version of the debugger (which, I understand is not the same; but I think it still uses CodeMirror) doesn't choke on syntax highlighting like the current version of the FF debugger.
I agree, but it's the fastest to recover from the regression. Once that's done we can look into upgrading codemirror and trying to address the performance issue.
Comment 15•2 years ago
|
||
Sounds good -- this has been blocking me using Firefox at work so that should hopefully help!
Assignee | ||
Comment 16•2 years ago
|
||
Disabling syntax highlighting does fix the issue, so this is not caused by something else (e.g. column breakpoints).
Assignee | ||
Comment 17•2 years ago
|
||
I need to check that again with a fresh brain, but I think this should be correct and seems to fix the perf issue:
function countColumn(string, end, tabSize, startIndex, startValue) {
// [...]
for (var i = startIndex || 0, n = startValue || 0;;) {
// slicing the string before checking the tab char index
var nextTab = string.substring(i, end).indexOf("\t");
if (nextTab < 0) { return n + (end - i) }
n += nextTab;
n += tabSize - (n % tabSize);
i = nextTab + 1;
}
}
the idea is that most of the time was spent in indexOf
, which makes sens as the string we're looking at is huge.
Getting just the token that needs to be checked makes it faster
Comment 18•2 years ago
|
||
Set release status flags based on info from the regressing bug 1754407
Updated•2 years ago
|
Assignee | ||
Comment 19•2 years ago
|
||
I tried to reproduce the issue in CodeMirror with the test case from Bug 1778758 , building this small app https://fx-devtools-codemirror5-long-lines.glitch.me/ that uses the exact same codeMirror bundle we're using in DevTools
Clicking the "Load big single-line file" button does fetch the single-line amo js file and put it in a codeMirror instance.
Unlike in DevTools, this happens quite fast:
- test app: ~2s https://share.firefox.dev/3zEIN7x
- DevTools debugger: ~20s https://share.firefox.dev/3zcqIfl
we must be doing something wrong here, maybe calling something too many times? I'll continue to investigate
Assignee | ||
Comment 20•2 years ago
|
||
This seems to be the culprit: https://searchfox.org/mozilla-central/rev/2bbb0c0a90df20702df8c8011a8996536a83cb75/devtools/client/debugger/src/utils/editor/source-documents.js#156-161
const doc = editor.createDocument();
setDocument(source.id, doc);
editor.replaceDocument(doc);
setEditorText(editor, source.id, content);
setMode(editor, source, content, symbols);
Creating a document, then setting the text and finally setting the mode seem to trigger some slow path.
Switching to something like
const doc = editor.createDocument(
getTextForDocument(source.id, content),
getModeForDocument(source, content, symbols)
);
setDocument(source.id, doc);
editor.replaceDocument(doc);
makes the operation run as fast as in my test app, with the test still being highlighted
Now for the regression, before, we would hit
https://searchfox.org/mozilla-central/rev/2bbb0c0a90df20702df8c8011a8996536a83cb75/devtools/client/debugger/src/utils/editor/source-documents.js#122-129
// Disable modes for minified files with 1+ million characters Bug 1569829
if (
content.type === "text" &&
isMinified(source, content) &&
content.value.length > 1000000
) {
return;
}
when trying to set the mode, so that's why the file wasn't highlighted.
We were bailing because isMinified
would return true, and now it returns false.
That's because (I guess) of changes in the data structure, while the code in isMinified
wasn't updated
https://searchfox.org/mozilla-central/rev/2bbb0c0a90df20702df8c8011a8996536a83cb75/devtools/client/debugger/src/utils/isMinified.js#21
https://searchfox.org/mozilla-central/rev/2bbb0c0a90df20702df8c8011a8996536a83cb75/devtools/client/debugger/src/utils/isMinified.js#18-24
if (
!sourceTextContent ||
!isFulfilled(sourceTextContent) ||
sourceTextContent.value.type !== "text"
) {
return false;
}
now type
is directly in sourceTextContent
, and sourceTextContent
doesn't seem to hold the state
property that is checked in isFulfilled
So I think we should fix isMinified
in this bug, and have another bug to be able to highlight bigger file, not setting the text and the mode separately.
Updated•2 years ago
|
Assignee | ||
Comment 21•2 years ago
|
||
Changes in Bug 1754407 caused a performance regression as it now triggered
syntax highlighting of big files, which we avoided before.
This is because we don't call isMinified
with the expected type of data, causing
the file to not be seen as minified, and thus highlighting it.
Updated•2 years ago
|
Assignee | ||
Comment 22•2 years ago
|
||
Depends on D153140
Assignee | ||
Comment 23•2 years ago
|
||
We'll re-enable syntax highlighting for big files in Bug 1782177
Comment 24•2 years ago
|
||
Comment 25•2 years ago
|
||
bugherder |
Comment 26•2 years ago
|
||
The patch landed in nightly and beta is affected.
:nchevobbe, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox104
towontfix
.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 27•2 years ago
|
||
(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #26)
The patch landed in nightly and beta is affected.
:nchevobbe, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox104
towontfix
.For more information, please visit auto_nag documentation.
sure, let me do that
Assignee | ||
Comment 28•2 years ago
|
||
Comment on attachment 9287600 [details]
Bug 1779173 - [devtools] Fix call to isMinified in Debugger to prevent performance issue when opening big single-line file. r=jdescottes.
Beta/Release Uplift Approval Request
- User impact if declined: Opening big minified files in the Debugger freezes the browser, even on beefy machine
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: 1. Go to https://addons.mozilla.com/
- Open the debugger
- Look for a file in
addons.mozilla.com > static-frontend
that looks likeamo-[numbers].js
, and open it
Expected results:
The color of the text should stay black, and the browser shouldn't freeze
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): DevTools bug, the patch only reverts a few changes
We are working on a performance test (https://phabricator.services.mozilla.com/D153141), that already confirms that the issue is fixed. - String changes made/needed:
- Is Android affected?: Yes
Assignee | ||
Updated•2 years ago
|
Comment 29•2 years ago
|
||
Comment on attachment 9287600 [details]
Bug 1779173 - [devtools] Fix call to isMinified in Debugger to prevent performance issue when opening big single-line file. r=jdescottes.
Approved for 104.0b5
Comment 30•2 years ago
|
||
bugherder uplift |
Updated•2 years ago
|
Assignee | ||
Comment 31•2 years ago
|
||
Comment on attachment 9287600 [details]
Bug 1779173 - [devtools] Fix call to isMinified in Debugger to prevent performance issue when opening big single-line file. r=jdescottes.
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: This bug makes it quite hard to debug big files, which are quite frequent nowadays.
- User impact if declined: Opening big minified files in the Debugger freezes the browser, even on beefy machine, making it quite painful to debug scripts.
- Fix Landed on Version: 105
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): DevTools bug, the patch only reverts a few changes
Assignee | ||
Updated•2 years ago
|
Comment 32•2 years ago
|
||
Reproduced the issue with Firefox 104.0a1(20220712215641) on Windows 10x64. The browser freezes after performing the steps from comment 28 and comment 0.
The issue is verified fixed with Firefox 105.0a1 (20220801215129) and 104.0b5 (20220801132524) from comment 30 on Windows 10x64, macOS 11 and Ubuntu 21. After following STR from comment 0 and comment 29 the browser no longer freezes and the text is written in black color.
However, switching dev tools tabs multiple times may lead to browser slowness and sometimes low time freezes. Should we file another issue for this and close this one as verified? Attached a screen recording. Thank you!
Assignee | ||
Comment 33•2 years ago
|
||
(In reply to Alexandru Trif, QA [:atrif] from comment #32)
Created attachment 9288069 [details]
1779173.gifReproduced the issue with Firefox 104.0a1(20220712215641) on Windows 10x64. The browser freezes after performing the steps from comment 28 and comment 0.
The issue is verified fixed with Firefox 105.0a1 (20220801215129) and 104.0b5 (20220801132524) from comment 30 on Windows 10x64, macOS 11 and Ubuntu 21. After following STR from comment 0 and comment 29 the browser no longer freezes and the text is written in black color.However, switching dev tools tabs multiple times may lead to browser slowness and sometimes low time freezes. Should we file another issue for this and close this one as verified? Attached a screen recording. Thank you!
yes, it might be different.
it would be great to have a bug with a profile showing the slowness
Updated•2 years ago
|
Comment 34•2 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #33)
yes, it might be different.
it would be great to have a bug with a profile showing the slowness
Thank you! Filled bug 1782682. Changing flags per comment 32 and comment 33.
Comment 35•2 years ago
|
||
Comment on attachment 9287601 [details]
Bug 1779173 - [devtools] Track opening of big files in custom debugger DAMP test. r=jdescottes.
Revision D153141 was moved to bug 1782701. Setting attachment 9287601 [details] to obsolete.
Comment 36•2 years ago
|
||
Comment on attachment 9287600 [details]
Bug 1779173 - [devtools] Fix call to isMinified in Debugger to prevent performance issue when opening big single-line file. r=jdescottes.
Approved for 102.2esr.
Comment 37•2 years ago
|
||
bugherder uplift |
Comment 38•2 years ago
|
||
Verified fixed with 102.2.0esr (20220802204048) from comment 37 on Windows 10x64, macOS 11 and Ubuntu 21. After following STR from comment 0 and comment 29 the browser and debugger no longer freeze and the text is written in black color.
Description
•