870794 - [snappy] View Source renders very slowly on large pages. Freezes Firefox for 5-45 seconds.

Status: VERIFIED FIXED

(Toolkit :: View Source, defect)

Description

[nemo]

So, I noticed this on an internal XHTML page, that, due to whitespace added by the tag lib, was 11,902 lines and 231,673 bytes.
after :g/^\s*$/d (which probably clobbered a few lines inside pre blocks but should be pretty close) it was down to 1,883 lines and 145,278 bytes.
Obviously when served over the network, compressed, the size is essentially the same.  19173 bytes vs 18047.

The 1,883 line file loads in about a second in View Source.

The 11,902 line file, which is mostly whitespace with nothing to syntax highlight, loads in ~15 seconds in View Source.

If I click on a link in View Source to jquery.flot.pie.js  - the most recent stable version of flot's pie chart formatter which is 752 lines of nicely formatted JS, it hangs, and eventualy loads in ~30 seconds.

Interestingly, there is absolutely no syntax highlighting that I can discern. Everything is black.

No idea what caused this behaviour, but Firefox View Source did not do this before.

The irritating thing for me is that while it is doing this, it completely locks up the entire browser.  If it did whatever it was doing in some worker or thread, it would be a bit less annoying. 

Could it be due to some new feature of View Source, like the fact that it now marks up invalid HTML?

Comment 1

[nemo]

"Interestingly, there is absolutely no syntax highlighting that I can discern. Everything is black."
That's in the slow to load JS file which appears like plain text in View Source.

The web page view source is formatted normally.

Could be due to bad content type on server and View Source not being as smart about IDing content as browser is when loading it (maybe even ignoring the fact that the <script> tag had type="text/javascript" set.

Comment 2

[nemo]

Added demo page.  Was used for stress testing some other silliness in the past, but View Source on it is a good demo of the problem for HTML files.  The JS library mentioned is a good demo of prob for other formats I guess.

Comment 3

[nemo]

Added link to a page that demonstrates the problem nicely.
Wait for page to fully load, hit ctrl-u.
It takes ~5s to render in Firefox 21 stable.
IT takes ~120 seconds to render in Firefox Nightly as of today,
during which time the browser is completely locked up and one core thrashes away.  At least if it is taking 120s to do... something... couldn't it do it in a worker thread? :(

Comment 4

[nemo]

Oh. Same behaviour in Beta BTW.  And, yeah, cross-platform.

Comment 5

[nemo]

*sigh* Sorry for bugspam.  That is, the *bad* behaviour exists in the Beta (the 2 minute plus view source time on the test page w/ total browser lockup)

Comment 6

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/9d5f05a6d497
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130409 Firefox/23.0 ID:20130409113132
Bad:
http://hg.mozilla.org/mozilla-central/rev/9db46ddfb517
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130409 Firefox/23.0 ID:20130409162448
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b4fec0759b12&tochange=c3008e662841

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f55d9235d5a0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130408 Firefox/23.0 ID:20130408203934
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f916756bbcbd
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130408 Firefox/23.0 ID:20130408233834
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f55d9235d5a0&tochange=f916756bbcbd


Regression window(m-aurora)
Good:
http://hg.mozilla.org/releases/mozilla-aurora/rev/487982220b4e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130502 Firefox/22.0 ID:20130502004015
Bad:
http://hg.mozilla.org/releases/mozilla-aurora/rev/3967ffd09b3c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130503 Firefox/22.0 ID:20130503004014
Pushlog:
http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=487982220b4e&tochange=3967ffd09b3c

Regressed by:
For m-central:
86a405c5f7af	Simon Montagu — The test for bidi isolated subparagraphs should be > 0, not > 1. Bug 859093, r=roc

For m-Aurora:
759b1354f8cd	Simon Montagu — The test for bidi isolated subparagraphs should be > 0, not > 1. Bug 859093, r=roc, a=akeybl

Comment 7

[Alice0775 White]

stack :
bp-a97c71cc-c8c1-4a81-941e-cb4e52130607

0 	xul.dll 	nsFrameList::InsertFrames 	layout/generic/nsFrameList.cpp:148
1 	xul.dll 	nsContainerFrame::InsertFrames 	layout/generic/nsContainerFrame.cpp:150
2 	xul.dll 	SplitInlineAncestors 	layout/base/nsBidiPresUtils.cpp:451
3 	xul.dll 	nsBidiPresUtils::ResolveParagraph 	layout/base/nsBidiPresUtils.cpp:904
4 	xul.dll 	nsBidiPresUtils::TraverseFrames 	layout/base/nsBidiPresUtils.cpp:1155
5 	xul.dll 	nsBidiPresUtils::TraverseFrames 	layout/base/nsBidiPresUtils.cpp:1163
6 	xul.dll 	nsBidiPresUtils::Resolve 	layout/base/nsBidiPresUtils.cpp:616
7 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:965
8 	xul.dll 	nsBlockReflowContext::ReflowBlock 	layout/generic/nsBlockReflowContext.cpp:266
9 	xul.dll 	nsBlockFrame::ReflowBlockFrame 	layout/generic/nsBlockFrame.cpp:3080
10 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:2013
11 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:1011
12 	xul.dll 	nsBlockReflowContext::ReflowBlock 	layout/generic/nsBlockReflowContext.cpp:266
13 	xul.dll 	nsBlockFrame::ReflowBlockFrame 	layout/generic/nsBlockFrame.cpp:3080
14 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:2013
15 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:1011
16 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:971
17 	xul.dll 	nsCanvasFrame::Reflow 	layout/generic/nsCanvasFrame.cpp:488
18 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:971
19 	xul.dll 	nsHTMLScrollFrame::ReflowScrolledFrame 	layout/generic/nsGfxScrollFrame.cpp:446
20 	xul.dll 	nsHTMLScrollFrame::ReflowContents 	layout/generic/nsGfxScrollFrame.cpp:544
21 	xul.dll 	nsHTMLScrollFrame::Reflow 	layout/generic/nsGfxScrollFrame.cpp:785
22 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:971
23 	xul.dll 	ViewportFrame::Reflow 	layout/generic/nsViewportFrame.cpp:226
24 	xul.dll 	PresShell::DoReflow 	layout/base/nsPresShell.cpp:7845
25 	xul.dll 	PresShell::ProcessReflowCommands 	layout/base/nsPresShell.cpp:7986
26 	xul.dll 	PresShell::FlushPendingNotifications 	layout/base/nsPresShell.cpp:3896
27 	xul.dll 	nsRefreshDriver::Tick 	layout/base/nsRefreshDriver.cpp:1184
28 	xul.dll 	nsTArray_Impl<nsRefPtr<nsRefreshDriver>,nsTArrayInfallibleAllocator>::AppendElem 	obj-firefox/dist/include/nsTArray.h:1044
29 	xul.dll 	nsTimerImpl::Fire 	xpcom/threads/nsTimerImpl.cpp:543
30 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:627
31 	xul.dll 	NS_ProcessNextEvent 	obj-firefox/xpcom/build/nsThreadUtils.cpp:238
32 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:82
33 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:212
34 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:186
35 	xul.dll 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:163
36 	xul.dll 	nsAppShell::Run 	widget/windows/nsAppShell.cpp:113
37 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:269
38 	xul.dll 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3851
39 	xul.dll 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3919
40 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:4132
41 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp:272
42 	firefox.exe 	NS_internal_main 	browser/app/nsBrowserApp.cpp:632
43 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:105
44 	firefox.exe 	__tmainCRTStartup 	crtexe.c:552
45 	kernel32.dll 	BaseThreadInitThunk 	
46 	ntdll.dll 	__RtlUserThreadStart 	
47 	ntdll.dll 	_RtlUserThreadStart

Comment 8

[bhavana bajaj [:bajaj]]

Tracking as this is Fx22 regression and passing onto :roc as Bug 859093 is the suspected regressing bug.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Backed out on beta.
https://hg.mozilla.org/releases/mozilla-beta/rev/366114b62435

Comment 10

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0

Verified as fixed in Firefox 22 beta 5 (buildID: 20130612084701).

Comment 11

[Simon Montagu :smontagu]

Attachment: Patch

I'm not certain why this is effective, but it definitely is: with a debug build the wall-clock loading time of the test case goes down from ~6 minutes to ~30 seconds, which is the same as with the backout, and the number of calls to CreateContinuingFrame() from SplitInlineAncestors() in nsBidiPresUtils.cpp goes down from 119208 to 0.

In any case removing this rule doesn't regress bug 751841, so apparently the comment is wrong and it was working around bug 859093 rather than bug 717811 as I thought at the time.

Comment 12

[nemo]

Hrm.  So, since you're just dropping some CSS.  Does this mean web pages w/ similar CSS could be impacted as well as View Source?

Comment 13

[Simon Montagu :smontagu]

Certainly they could (but "similar content and CSS" would be more accurate). With the old HTML parser I used to know how to create an HTML page that reproduced the content of view-source, but I don't know how to do that, or if it's even possible, with the new HTML5 parser.

Comment 14

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50e5eb0c4008

Comment 15

[Simon Montagu :smontagu]

I filed bug 885689 to follow up on the underlying performance problems. Just to wrap up a couple of open questions here:

The reason why the patch is effective is because the U+200E character is classified as a bidi control character and turns on bidi processing, so without it we don't go through the bidi resolution code path at all. This means of course that the problem still exists with view-source of a page containing RTL text.

In answer to my own question about creating an HTML page with the same content as view-source of the test case, it can be simply done by opening view-source:m8y.org/tmp/testcase164.html in browser and Save As.

Comment 16

[Simon Montagu :smontagu]

bugzilla didnt linkify that well, try 
view-source:http://m8y.org/tmp/testcase164.html

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/50e5eb0c4008

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Uplift nomination for Beta?

Comment 19

[Simon Montagu :smontagu]

Comment on attachment 765528 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 859093
User impact if declined: Bad performance regression in view source
Testing completed (on m-c, etc.): baked on m-c since 2013-06-20 and on Aurora since 2013-06-24
Risk to taking this patch (and alternatives if risky): Minimal, the patch just removes a workaround for a bug that no longer exists
String or IDL/UUID changes made by this patch: None

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a25d72a744ee

Comment 21

[u279076]

Bogdan, please test to verify this is fixed in Firefox 23 and 24. Thank you.

Comment 22

[Cornel Ionce [:noni] [Hubs QA]]

Verified as fixed on Firefox 23 Beta 5 (build ID:20130711122148) and on Aurora (build ID: 20130711004005).

User Agents:
(Beta)
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0

(Aurora)
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130711 Firefox/24.0