Closed Bug 1419519 Opened 7 years ago Closed 6 years ago

Firefox Quantum lags when editing files on GitHub.

Categories

(Core :: Layout: Text and Fonts, defect, P3)

Product:
Core
Component:
Layout: Text and Fonts
Version:
58 Branch
Type:
defect

Tracking

()

Status:
RESOLVED DUPLICATE of bug 1449828
Project Flags:
Performance Impact high

People

(Reporter: BladeMight, Unassigned)

References

Details

(Keywords: perf)

Attachments

(1 file)

spellscheck.diff
2.78 KB, patch
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20171121100129

Steps to reproduce:

1. You have to have GitHub account.
2. Start editing any file(mine was 528 lines long, and 33Kb).
3. CPU usage will be always at 25~30% and it will lag when you input something.



Actual results:

Editing files on GitHub is nearly impossible in Firefox Quantum, high CPU usage and lag on every character input.



Expected results:

Not to lag, at least. And smaller CPU usage, if possible.

Additional details:  
  CPU: 4x2.3Ghz
  GPU: 1GB GDDR5
  RAM: 6GB 
  OS: Windows 7 x64
  Firefox: 58 / Nigtly
Please troubleshoot with Safe Mode: https://support.mozilla.org/kb/troubleshoot-firefox-issues-using-safe-mode
Component: Untriaged → Editor
Product: Firefox → Core
(In reply to Kohei Yoshino [:kohei] from comment #1)
> Please troubleshoot with Safe Mode:
> https://support.mozilla.org/kb/troubleshoot-firefox-issues-using-safe-mode

Same, the more i write the more it lags(from ~500ms to ~2000-3000ms after each key press)...
FWIW I ran into janky typing on github recently as well.  I don't have any addons installed.  I did not have time to take a profile, though.

Mark for [qf] triage as typing jank is very noticeable to users.
Whiteboard: [qf]
Priority: -- → P3
Keywords: perf
Profiling using over 14000 lines, and child process is mostly just waiting for me input and parent process doesn't really much.
Max event processing delay I see is <100ms.

bkelly, (or anyone) if you get a chance to create a performance profile, that would be great.



https://perf-html.io/ has the addon for profiling.
Flags: needinfo?(bkelly)
Couldn't see anything editor related in the profile, more like JS under event listeners.
But we need some profile actually showing the issue.
Component: Editor → General
I haven't seen this recently.  Its possible I was suffering from a session that was leaking due to another bug.  If I see it again I'll grab a profile.
Flags: needinfo?(bkelly)
I can still reproduce it on latest nightly, with all addons disabled and in safe mode.
BladeMight, could you please create a performance profile using https://perf-html.io/
https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Reporting_a_Performance_Problem#Setting_up_the_Gecko_Profiler_extension may be useful documentation.
Flags: needinfo?(BladeMight)
Here you go :smaug
http://dropmefiles.com/fT3kh
Flags: needinfo?(BladeMight)
Hmm, that is missing symbols. Could you perhaps create a new one and use the "Share..." functionality in the profiler UI?
Flags: needinfo?(BladeMight)
[:smaug] Here: https://perfht.ml/2DAg8Ff
Flags: needinfo?(BladeMight)
Thanks. Content process 5 looks like github, and there spellchecking is taking time.

Just for testing, is the behavior better if you disable spellchecking?
Component: General → Spelling checker
Whiteboard: [qf] → [qf:p1][qf:f60]
[:smaug] yeah it is really better without spellchecking, with it disabled no slowdown at all. Here profile with spellchecking disabled:
https://perf-html.io/public/7ed9d4ab43b1429f41a4f534cdc353d38de2fddb/calltree/?hiddenThreads=&thread=2&threadOrder=0-2-3-4-5-6-8-1-7&v=2
dupe of bug 1303749?
See Also: → 1422624
Nope, not a dup of that, as far as I see.
I still don't know how to reproduce this.


BladeMight, by any chance do you have some minimal testcase I could try?
(with clear steps to reproduce)
Flags: needinfo?(BladeMight)
[:smaug] It happens when i editing WIKI pages on GitHub, for example here: 
https://github.com/ygini/Hello-IT/wiki/public.test.http/_edit
When you write more with spellchecker enabled you will see how it will slowdown.
Here a screenshot of edit box in which slowdown are:
https://i.imgur.com/ZpXsk4g.jpg

Steps to reproduce:

1. Go to wiki edit page.
2. Start writing something in the edit box.(on the screen abowe)
3. See how it will slowdown the more you wrote. (if spellchecking enabled)
Flags: needinfo?(BladeMight)
Any updates?
I don't see any slow downs on my machine, but perhaps this is too fast machine. But I do get similar profile, which should help analyzing this.
Looking forward to finding out why is that.
Jonathan, is there anything we could do to nsTextFrame::SetSelectedRange?
See the profile in comment 12, content process 5. CombineSelectionUnderlineRect underneath it takes lots of time because of EnsureTextRun call. Perhaps EnsureTextRun could be optimized?
Component: Spelling checker → Layout: Text
Flags: needinfo?(jfkthame)
hmm, bug 1354641 looks a bit weird.
But that shouldn't affect here.


Reporter, I pushed a patch to tryserver. Could you perhaps try it?

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/f869055908ae914f4fbe5fcb644c5433c9f64821
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=f869055908ae914f4fbe5fcb644c5433c9f64821
remote: recorded changegroup in replication log in 0.079s


Once the pgo build is ready, is should show up somewhere in https://archive.mozilla.org/pub/firefox/try-builds/
Look for f869055908ae there.
Attached patch spellscheck.diff — — Splinter Review 
This is the patch I pushed to try. It should basically make spellchecking to happen less frequently. We still probably need improvements to textrun creation.
(In reply to Olli Pettay [:smaug] from comment #24)

> Reporter, I pushed a patch to tryserver. Could you perhaps try it?

Yes, i'll try once it finish building.
Hmm, perhaps https://queue.taskcluster.net/v1/task/d97ksFIHScC3FGpRhHpDxQ/runs/0/artifacts/public/build/target.zip is the right link.
It is target.zip from pgo [B] in tryserver.

Looks like there are couple of testfailures, but those look like buggy tests.
So now it scans only some word i wrote and words near, and in background for all text, but it still slow!!!
Also i noticed that if i switch spellchecker dictionary to Russian, downloaded from here:
  https://addons.mozilla.org/en-US/firefox/addon/russian-spellchecking-dic-3703/
it will become slowly slower, than with just english!!! And with this dictionary over english text it becomes extremly slow... Here another profile:
https://perf-html.io/public/43d03929b3b7ef6558c554cbf09003a9bc5293d4/calltree/?hiddenThreads=&thread=2&threadOrder=0-2-3-4-5-6-8-1-7&v=2
Seems dictionary which creates more highlights under wrong words(wavy line) makes it to be slower.
That profile seems to be missing symbols. ... which may make sense if it was done using the tryserver build.

I still don't understand how you get so long pauses as what comment 12 indicates. 
That is like 100x slower what I see locally (testing Nightly). I tested on linux and windows.
In your profile there is SetupLineBreakerContext call which I don't see locally on my profiles.
Need some feedback from jfkthame here
It seems that the problem arises when SetSelectedRange causes the content to be reflowed; and that happens if the spell-check underline requires enlarging the overflow area of the frame, because we end up marking the textframe(s) as dirty here:

https://searchfox.org/mozilla-central/rev/eeb7190f9ad6f1a846cd6df09986325b3f2c3117/layout/generic/nsTextFrame.cpp#7687-7691

I think we must end up collecting the entire textarea's text, or at least all the text up to the line where the selection is being set, to feed to the line-breaker. That gets expensive in a textarea with lots of content. :( Seems like we should be able to improve that, and keep any reflow more local to the immediate line(s) that are affected.

The other thing that seems unfortunate is that we're triggering reflow -- re-doing linebreaking, etc -- although setting a selected range, AFAIK, cannot actually affect line breaks, because the style properties that are usable on a selection pseudo-element are properties that have no effect on measurement. (Color, text-decoration, etc.) So it would be nice if we had a method to just update overflow areas without doing the rest of reflow. Currently, we're acting as though an arbitrary style change has happened, but setting a selection range is actually a highly restricted change.

How bad this is will depend on various factors. On macOS, the spell-check underline style is dotted, which is much less likely to trigger this behavior than the wavy style used on other platforms. But font size and style differences could also be a factor; with a larger font size, perhaps even the wavy underline wouldn't lead to expanding the overflow area and hence triggering the reflow here.

As an experiment, I've pushed a try build https://treeherder.mozilla.org/#/jobs?repo=try&revision=454546b6c20db5b18594c229ad9bce90cda0ca39 where the spell-check underline style on Windows and Linux should be changed from wavy to dotted. Reporter, I'd be interested to know whether this makes a significant difference for you. The Windows build will be available at https://queue.taskcluster.net/v1/task/eUwVredzRwueJzn2R7LutA/runs/0/artifacts/public/build/target.zip.
Flags: needinfo?(jfkthame) → needinfo?(BladeMight)
(In reply to Jonathan Kew (:jfkthame) from comment #30)
> Reporter, I'd be interested to know whether this makes a
> significant difference for you.

It seems what you've done removed slowdown on when i type, but now it checks only whole text once(on dictionary switch/page reload), and not checks the words i write right away. 

Can't we make spellchecker to just take and check at least line that is in tex tarea when writing(inputting), and whole text on some operations like Cut/Paste. Or even better just get text that is visible right now and check only it, on scroll etc. events(probably better on end of the events) update and recheck.
Flags: needinfo?(BladeMight)
Whiteboard: [qf:p1][qf:f60] → [qf:f61][qf:p1]
See also bug 1444742.
BladeMight, does this still occurs with layout.display-list.retain=false (by about:config) ?
Flags: needinfo?(BladeMight)
Yes, it issue still persists even with that setting set to false. Writing one line on github wiki edit in my case is enough to catch slowdown between keypresses.
Flags: needinfo?(BladeMight)
Depends on: 1449828
If this occurs on 58+, this might be regression by bug 1403521.  But some case will be fixed by bug 1449828. (I don't know how to reproduce this case)
[:m_kato] See: comment #18 and comment #28, there i write clear when and after what it happens.
(In reply to BladeMight from comment #37)
> [:m_kato] See: comment #18 and comment #28, there i write clear when and
> after what it happens.

BladeMight, I cannot reproduce this (using comment #18) on the latest Nightly since I land bug 1449828.  Does this still occurs on the latest Firefox Nightly?

Bug 1449828 will be landed in 60.
Flags: needinfo?(BladeMight)
[:m_kato] Yeah, now it is fine. Thanks for fixing this.
Flags: needinfo?(BladeMight)
Thanks for verified!
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
No longer depends on: 1449828
Resolution: --- → DUPLICATE
Performance Impact: --- → P1
Whiteboard: [qf:f61][qf:p1]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: