1294161 - Poor performance when viewing washingtonpost.com comments

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[q1]

FF 47.0 performs poorly when viewing comments on www.washingtonpost.com stories after pausing the comment stream for a few minutes and then resuming the comment stream, if many comments have been added in the interregnum.

When this occurs, the "Warning: Unresponsive script" dialog appears, and when you click "Continue", a few comments appear, followed by another "Unresponsive script" dialog, continuing until you run out of patience. When FF is in this state, scrolling the comments is very slow, and memory consumption increases to 1 GB+.

It is very difficult to close the offending tab or to close FF when it is in this state; generally you have to kill it from the task manager.

To reproduce:

1. Visit http://www.washingtonpost.com;
2. Click on a popular story;
3. Click on the "xxxx comments" box at the story's bottom;
4. Wait for the comments to appear. The story is suitable if new comments appear every few seconds or faster;
5. Pause the comment stream by clicking "Pause live updates";
6. Wait >= 5 minutes. Spend the time scrolling down and reading existing comments;
7. Scroll back to the top of the comment stream and click "Resume live updates";
8. Experience the problem.

I am using ABP 2.7.3, but no other add-ons. I have had this problem as long as I've been reading washingtonpost.com comment threads, which is at least since FF 33.0 (with correspondingly old ABP versions).

Comment 1

[Abe - QA (:Abe_LV)]

Hi Reporter[q1],

I tested this on windows xp with latest nightly and release-47 including the ABP addon. The page is responsive and I am not getting any dialogues.  
Can you provide "about:support" data as attachment? 

Thanks

Comment 2

[q1]

(In reply to Abe - QA from comment #1)
> Hi Reporter[q1],
> 
> I tested this on windows xp with latest nightly and release-47 including the
> ABP addon. The page is responsive and I am not getting any dialogues.  
> Can you provide "about:support" data as attachment? 
> 
> Thanks

Since filing this bug, I upgraded to FF 48.0, which -- so far -- exhibits much better behavior on WaPo comments. Here is the about:support from FF 48.0:

----------------------------------------------------
This page contains technical information that might be useful when you're trying to solve a problem. If you are looking for answers to common questions about Firefox, check out our support website.

Application Basics
Name 	Firefox
Version 	48.0
Build ID 	20160726073904
Update History 	
Update Channel 	release
User Agent 	Mozilla/5.0 (Windows NT 5.1; rv:48.0) Gecko/20100101 Firefox/48.0
OS 	Windows_NT 5.1 x86
Profile Folder 	
Enabled Plugins 	about:plugins
Build Configuration 	about:buildconfig
Memory Use 	about:memory
Registered Service Workers 	about:serviceworkers
Multiprocess Windows 	0/10 (Disabled by add-ons)
Safe Mode 	false
Profiles 	about:profiles
Crash Reports for the Last 3 Days
Report ID 	Submitted

This application has not been configured to display crash reports.
Extensions
Name 	Version 	Enabled 	ID
Adblock Plus	2.7.3	true	{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}
Firefox Hello	1.4.3	true	[e-mail]
Multi-process staged rollout	1.0	true	[e-mail]
Pocket	1.0.4	true	[e-mail]
Microsoft .NET Framework Assistant	0.0.0	false	{20a82645-c095-46ed-80e3-08825760534b}
Graphics
Features
Compositing	Direct3D 9
Asynchronous Pan/Zoom	none
WebGL Renderer	Google Inc. -- ANGLE (AMD Radeon HD 7570M Direct3D9 vs_3_0 ps_3_0)
Hardware H264 Decoding	No; Failed to create H264 decoder
DirectWrite	false (0.0.0.0)
GPU #1
Active	Yes
Description	AMD Radeon HD 7570M
Vendor ID	0x1002
Device ID	0x6841
Driver Version	8.950.0.0
Driver Date	2-14-2012
Drivers	ati2dvag
Subsys ID	00011179
RAM	Unknown
Diagnostics
AzureCanvasAccelerated	0
AzureCanvasBackend	skia
AzureContentBackend	cairo
AzureFallbackCanvasBackend	cairo
failures	[GFX1-]: [D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
Failure Log
(#0) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#117) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#118) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#119) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#120) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#121) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#122) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#123) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#124) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#125) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#126) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#127) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#128) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#129) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#130) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
(#131) Error	[D3D9] DataTextureSourceD3D9::UpdateFromTexture failed
Important Modified Preferences
Name 	Value accessibility.typeaheadfind.flashBar	0
browser.cache.disk.capacity	358400
browser.cache.disk.filesystem_reported	1
browser.cache.disk.hashstats_reported	1
browser.cache.disk.smart_size.first_run	false
browser.cache.disk.smart_size.use_old_max	false
browser.cache.frecency_experiment	3
browser.download.importedFromSqlite	true
browser.fixup.alternate.enabled	false
browser.places.smartBookmarksVersion	8
browser.search.searchEnginesURL	
browser.search.suggest.enabled	false
browser.search.update	false
browser.sessionstore.enabled	true
browser.sessionstore.upgradeBackup.latestBuildID	20150917150946
browser.startup.homepage	about:blank
browser.startup.homepage_override.buildID	20160726073904
browser.startup.homepage_override.mstone	48.0
browser.tabs.drawInTitlebar	false
browser.tabs.remote.autostart.2	true
dom.apps.reset-permissions	true
dom.ipc.plugins.flash.subprocess.crashreporter.enabled	false
dom.ipc.plugins.reportCrashURL	false
dom.mozApps.signed_apps_installable_from	
dom.mozApps.used	true
dom.push.connection.enabled	false
dom.push.serverURL	
extensions.lastAppVersion	48.0
font.internaluseonly.changed	false
general.autoScroll	false
gfx.crash-guard.d3d11layers.appVersion	48.0
gfx.crash-guard.d3d11layers.deviceID	0x6841
gfx.crash-guard.d3d11layers.driverVersion	8.950.0.0
gfx.crash-guard.d3d11layers.feature-d2d	true
gfx.crash-guard.d3d11layers.feature-d3d11	true
gfx.crash-guard.glcontext.gfx.driver-init.direct3d11-angle	true
gfx.crash-guard.glcontext.gfx.driver-init.webgl-angle	true
gfx.crash-guard.glcontext.gfx.driver-init.webgl-angle-force-d3d11	false
gfx.crash-guard.glcontext.gfx.driver-init.webgl-angle-force-warp	false
gfx.crash-guard.glcontext.gfx.driver-init.webgl-angle-try-d3d11	true
gfx.crash-guard.status.d3d11layers	2
gfx.crash-guard.status.glcontext	2
gfx.driver-init.appVersion	42.0
gfx.driver-init.deviceID	0x6841
gfx.driver-init.driverVersion	8.950.0.0
gfx.driver-init.feature-d2d	true
gfx.driver-init.feature-d3d11	true
gfx.driver-init.status	2
keyword.enabled	false
media.autoplay.enabled	false
media.gmp-gmpopenh264.autoupdate	false
media.gmp-manager.buildID	20160604131506
media.gmp-manager.url	
media.gmp.storage.version.observed	1
media.hardware-video-decoding.failed	true
media.peerconnection.default_iceservers	
media.webrtc.debug.aec_log_dir	c:\DOCUME~1\browser\LOCALS~1\temp
media.webrtc.debug.log_file	c:\DOCUME~1\browser\LOCALS~1\temp\WebRTC.log
network.cookie.cookieBehavior	1
network.cookie.prefsMigrated	true
network.http.sendRefererHeader	0
network.http.sendSecureXSiteReferrer	false
network.http.spdy.allow-push	false
network.predictor.cleaned-up	true
places.database.lastMaintenance	1471365342
places.history.expiration.transient_current_max_pages	86634
plugin.disable_full_page_plugin_for_types	application/pdf
plugin.importedState	true
plugins.update.url	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_bgcolor	false
print.printer_HP_LaserJet_2300_Series_PCL_6.print_bgimages	false
print.printer_HP_LaserJet_2300_Series_PCL_6.print_colorspace	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_command	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_downloadfonts	false
print.printer_HP_LaserJet_2300_Series_PCL_6.print_duplex	1515870810
print.printer_HP_LaserJet_2300_Series_PCL_6.print_edge_bottom	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_edge_left	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_edge_right	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_edge_top	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_evenpages	true
print.printer_HP_LaserJet_2300_Series_PCL_6.print_footercenter	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_footerleft	&PT
print.printer_HP_LaserJet_2300_Series_PCL_6.print_footerright	&D
print.printer_HP_LaserJet_2300_Series_PCL_6.print_headercenter	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_headerleft	&T
print.printer_HP_LaserJet_2300_Series_PCL_6.print_headerright	&U
print.printer_HP_LaserJet_2300_Series_PCL_6.print_in_color	true
print.printer_HP_LaserJet_2300_Series_PCL_6.print_margin_bottom	0.5
print.printer_HP_LaserJet_2300_Series_PCL_6.print_margin_left	0.5
print.printer_HP_LaserJet_2300_Series_PCL_6.print_margin_right	0.5
print.printer_HP_LaserJet_2300_Series_PCL_6.print_margin_top	0.5
print.printer_HP_LaserJet_2300_Series_PCL_6.print_oddpages	true
print.printer_HP_LaserJet_2300_Series_PCL_6.print_orientation	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_page_delay	50
print.printer_HP_LaserJet_2300_Series_PCL_6.print_paper_data	-1
print.printer_HP_LaserJet_2300_Series_PCL_6.print_paper_height	279.00
print.printer_HP_LaserJet_2300_Series_PCL_6.print_paper_name	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_paper_size_type	1
print.printer_HP_LaserJet_2300_Series_PCL_6.print_paper_size_unit	1
print.printer_HP_LaserJet_2300_Series_PCL_6.print_paper_width	215.00
print.printer_HP_LaserJet_2300_Series_PCL_6.print_plex_name	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_resolution	600
print.printer_HP_LaserJet_2300_Series_PCL_6.print_resolution_name	
print.printer_HP_LaserJet_2300_Series_PCL_6.print_reversed	false
print.printer_HP_LaserJet_2300_Series_PCL_6.print_scaling	1.00
print.printer_HP_LaserJet_2300_Series_PCL_6.print_shrink_to_fit	true
print.printer_HP_LaserJet_2300_Series_PCL_6.print_to_file	false
print.printer_HP_LaserJet_2300_Series_PCL_6.print_unwriteable_margin_bottom	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_unwriteable_margin_left	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_unwriteable_margin_right	0
print.printer_HP_LaserJet_2300_Series_PCL_6.print_unwriteable_margin_top	0
print.printer_Microsoft_XPS_Document_Writer.print_bgcolor	false
print.printer_Microsoft_XPS_Document_Writer.print_bgimages	false
print.printer_Microsoft_XPS_Document_Writer.print_duplex	1515870810
print.printer_Microsoft_XPS_Document_Writer.print_edge_bottom	0
print.printer_Microsoft_XPS_Document_Writer.print_edge_left	0
print.printer_Microsoft_XPS_Document_Writer.print_edge_right	0
print.printer_Microsoft_XPS_Document_Writer.print_edge_top	0
print.printer_Microsoft_XPS_Document_Writer.print_evenpages	true
print.printer_Microsoft_XPS_Document_Writer.print_footercenter	
print.printer_Microsoft_XPS_Document_Writer.print_footerleft	&PT
print.printer_Microsoft_XPS_Document_Writer.print_footerright	&D
print.printer_Microsoft_XPS_Document_Writer.print_headercenter	
print.printer_Microsoft_XPS_Document_Writer.print_headerleft	&T
print.printer_Microsoft_XPS_Document_Writer.print_headerright	&U
print.printer_Microsoft_XPS_Document_Writer.print_in_color	true
print.printer_Microsoft_XPS_Document_Writer.print_margin_bottom	0.5
print.printer_Microsoft_XPS_Document_Writer.print_margin_left	0.5
print.printer_Microsoft_XPS_Document_Writer.print_margin_right	0.5
print.printer_Microsoft_XPS_Document_Writer.print_margin_top	0.5
print.printer_Microsoft_XPS_Document_Writer.print_oddpages	true
print.printer_Microsoft_XPS_Document_Writer.print_orientation	0
print.printer_Microsoft_XPS_Document_Writer.print_page_delay	50
print.printer_Microsoft_XPS_Document_Writer.print_paper_data	1
print.printer_Microsoft_XPS_Document_Writer.print_paper_height	11.00
print.printer_Microsoft_XPS_Document_Writer.print_paper_name	
print.printer_Microsoft_XPS_Document_Writer.print_paper_size_type	0
print.printer_Microsoft_XPS_Document_Writer.print_paper_size_unit	0
print.printer_Microsoft_XPS_Document_Writer.print_paper_width	8.50
print.printer_Microsoft_XPS_Document_Writer.print_resolution	1515870810
print.printer_Microsoft_XPS_Document_Writer.print_reversed	false
print.printer_Microsoft_XPS_Document_Writer.print_scaling	1.00
print.printer_Microsoft_XPS_Document_Writer.print_shrink_to_fit	true
print.printer_Microsoft_XPS_Document_Writer.print_to_file	false
print.printer_Microsoft_XPS_Document_Writer.print_unwriteable_margin_bottom	0
print.printer_Microsoft_XPS_Document_Writer.print_unwriteable_margin_left	0
print.printer_Microsoft_XPS_Document_Writer.print_unwriteable_margin_right	0
print.printer_Microsoft_XPS_Document_Writer.print_unwriteable_margin_top	0
print.printer_Samsung_M2070_Series.print_bgcolor	false
print.printer_Samsung_M2070_Series.print_bgimages	false
print.printer_Samsung_M2070_Series.print_duplex	1515870810
print.printer_Samsung_M2070_Series.print_edge_bottom	0
print.printer_Samsung_M2070_Series.print_edge_left	0
print.printer_Samsung_M2070_Series.print_edge_right	0
print.printer_Samsung_M2070_Series.print_edge_top	0
print.printer_Samsung_M2070_Series.print_evenpages	true
print.printer_Samsung_M2070_Series.print_footercenter	
print.printer_Samsung_M2070_Series.print_footerleft	&PT
print.printer_Samsung_M2070_Series.print_footerright	&D
print.printer_Samsung_M2070_Series.print_headercenter	
print.printer_Samsung_M2070_Series.print_headerleft	&T
print.printer_Samsung_M2070_Series.print_headerright	&U
print.printer_Samsung_M2070_Series.print_in_color	true
print.printer_Samsung_M2070_Series.print_margin_bottom	0.5
print.printer_Samsung_M2070_Series.print_margin_left	0.5
print.printer_Samsung_M2070_Series.print_margin_right	0.5
print.printer_Samsung_M2070_Series.print_margin_top	0.5
print.printer_Samsung_M2070_Series.print_oddpages	true
print.printer_Samsung_M2070_Series.print_orientation	0
print.printer_Samsung_M2070_Series.print_page_delay	50
print.printer_Samsung_M2070_Series.print_paper_data	1
print.printer_Samsung_M2070_Series.print_paper_height	11.00
print.printer_Samsung_M2070_Series.print_paper_name	
print.printer_Samsung_M2070_Series.print_paper_size_type	0
print.printer_Samsung_M2070_Series.print_paper_size_unit	0
print.printer_Samsung_M2070_Series.print_paper_width	8.50
print.printer_Samsung_M2070_Series.print_resolution	1515870810
print.printer_Samsung_M2070_Series.print_reversed	false
print.printer_Samsung_M2070_Series.print_scaling	1.00
print.printer_Samsung_M2070_Series.print_shrink_to_fit	true
print.printer_Samsung_M2070_Series.print_to_file	false
print.printer_Samsung_M2070_Series.print_unwriteable_margin_bottom	0
print.printer_Samsung_M2070_Series.print_unwriteable_margin_left	0
print.printer_Samsung_M2070_Series.print_unwriteable_margin_right	0
print.printer_Samsung_M2070_Series.print_unwriteable_margin_top	0
privacy.cpd.offlineApps	true
privacy.cpd.siteSettings	true
privacy.donottrackheader.enabled	true
privacy.sanitize.migrateFx3Prefs	true
privacy.sanitize.timeSpan	0
privacy.trackingprotection.introURL	
security.OCSP.enabled	0
security.ssl.errorReporting.url	
services.sync.declinedEngines	
storage.vacuum.last.index	1
storage.vacuum.last.places.sqlite	1469030338
Important Locked Preferences
	Name 	Value
JavaScript
Incremental GC 	true
Accessibility
Activated 	false
Prevent Accessibility 	0
Library Versions
	Expected minimum version	Version in use
NSPR	4.12	4.12
NSS	3.24 Basic ECC	3.24 Basic ECC
NSSSMIME	3.24 Basic ECC	3.24 Basic ECC
NSSSSL	3.24 Basic ECC	3.24 Basic ECC
NSSUTIL	3.24	3.24
Experimental Features
Name 	ID 	Description 	Active 	End Date 	Homepage 	Branch

Comment 3

[q1]

Ack, the problem is back, on FF 48.0 on https://www.washingtonpost.com/news/post-politics/wp/2016/08/17/trump-reshuffles-staff-in-his-own-image/#comments right now. Follow the reproduction directions in comment 0, but leave comments paused for >= 20 min.

Comment 4

[Abe - QA (:Abe_LV)]

Attachment: unresponsive.PNG

Using the url provided at comment 3, the browser displays "Warning: unresponsive script" dialog when the update is set to live from paused.

It takes sometime to reproduce this. First go to the article and pause the live update. After awhile, set the paused update to Live. Then the dialog will appear. The browser becomes very slow after clicking the buttons. 

---
Version 	51.0a1
Build ID 	20160817030202
Update Channel 	nightly
User Agent 	Mozilla/5.0 (Windows NT 5.1; rv:51.0) Gecko/20100101 Firefox/51.0

Comment 5

[Andrew Overholt [:overholt]]

Can one of you that can reproduce please get a profile? https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Reporting_a_Performance_Problem

Comment 6

[q1]

(In reply to Andrew Overholt [:overholt] from comment #5)
> Can one of you that can reproduce please get a profile?
> https://developer.mozilla.org/en-US/docs/Mozilla/Performance/
> Reporting_a_Performance_Problem

The browser does not respond to clicking the profiler icon when the problem is occurring. I have obtained a Visual Studio profiler trace while the problem is occurring under FF 48.0.1 instead, which I will answer questions about, but (for privacy reasons) not upload. Here are the top consumers of CPU while the problem is occurring:

   js::jit::JitcodeGlobalTable::searchInternal                                       17.16%
   `js::irregexp::RegExpEmpty::GetInstance'::`2'::dynamic atexit                     12.10%
      destructor for 'instance''
   [unknown]                                                                          7.30%
   EnumerateNativeProperties                                                          4.69%
   ProfileBuffer::addTag                                                              2.64%
   js::jit::JitcodeGlobalTable::lookupInternal                                        2.10%
   SelectorMatches                                                                    1.81%
   js::jit::JitProfilingFrameIterator::moveToNextFrame                                1.24%
   mergeStacksIntoProfile                                                             1.13%
   JS::ProfilingFrameIterator::getPhysicalFrameAndEntry                               1.08%

I don't understand why VS cannot determine what "unknown" is. I have loaded all FF symbols and virtually all Windows symbols.

In any case, the bug is very easy to reproduce. Just make sure to select an article that's accumulating comments quickly. Leave the comment stream paused for an hour, and you'll probably see the bug every time.

Comment 7

[q1]

Expanding some of the "unknowns" shows lots of time spent in reflow functions, along with very deep recursion involving such functions as nsBlockFrame::Reflow().

Comment 8

[q1]

Ah, "[unknown]" is probably the sole module with a VS "failed to load symbols" message: nvwgf2umx.dll, which is part of the NVIDIA video driver stack.

Comment 9

[Abe - QA (:Abe_LV)]

The profiler does not seem to be working in this case. To reproduce the issue, the live comments should be paused for few hours (at least for my case). 
Screen capture is here: https://testing-1.tinytake.com/sf/OTI2NjAzXzM5MTIyNDc

Comment 10

[Nika Layzell [:nika] (ni? for response)]

I left wapo running for a couple hours and noticed laggyness before even unpausing the comments. I took a profile. It seems like the cause of this slowdown is this function being run on a timer in an iframe:

        function S(e) {
            var t, n, r, i, o = vt.body;
            if (vt.getElementById("cSzl") ? (t = vt.getElementById("cSzl"), nIframe = !1) : (t = vt.createElement("iframe"), nIframe = !0, t.id = "cSzl"), !t) return e;
            t.style.position = "absolute", t.style.top = "-32000px";
            var c = 'function m(a,b,c,d){var e;return a.sheet.insertRule("@media ("+c+":"+d+") {div {text-decoration: underline} }",0),e=getComputedStyle(b,null).textDecoration=="underline",a.sheet.deleteRule(0),e}function bs(a,b,c,d,e,f,g,h){var i=(e+f)/2;return g==0||f-e<h?i:m(a,b,c,i+d)?bs(a,b,c,d,i,f,g-1,h):bs(a,b,c,d,e,i,g-1,h)}',
                a = 'var doc=document,s=doc.createElement("style");n=doc.createElement("div"),doc.body.appendChild(s);var zoom=bs(s,n,"min--moz-device-pixel-ratio","",.1,10,15,1e-4);';
            if (1 == nIframe && o.appendChild(t)) try {
                (n = t.contentDocument) && (n.open(), n.writeln("<!DOCTYPE html><html><head><title></title></head><body></body></html>"), n.close(), (i = n.createElement("script")) && (i.type = "text/javascript", i.text = c + a, n.body.appendChild(i), (r = t.contentWindow) && (e = r.zoom || e), n.body.removeChild(i)))
            } catch (s) {} else try {
                (n = t.contentDocument) && (i = n.createElement("script")) && (i.text = c + a, n.body.appendChild(i), (r = t.contentWindow) && (e = r.zoom || e), n.body.removeChild(i))
            } catch (s) {}
            return e
        }

This is the profile:
https://cleopatra.io/#report=cb0528999f567a7695b276b13d5488c4586203ff&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A4208755,%22end%22%3A4235718%7D,%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A4231809,%22end%22%3A4235527%7D,%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A4231902,%22end%22%3A4232865%7D%5D

It seems like the majority of the time is spent inserting an empty <style> tag into the document. I think that the amount of time it takes is probably related to how each time this function is run it seems to me like it adds a style tag to the iframe's document, and never removes it.


I then unpaused and resumed the comments, capturing this profile after it hung for a little bit (it seemed to hang less long for me, than it did for the reporter). This is the profile: https://cleopatra.io/#report=259e1bc763c42e69e922d32d295891cdd404bacd&filter=%5B%7B%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A10346063,%22end%22%3A10349397%7D%5D&selection=0,3525,367,177,3550,3551,3552,3553,3554,3555,3602

It seems like the time is coming from the same place, that is, this function S() I have pretty printed above.

Smaug, overholt suggested you might have ideas for how to continue from this.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

We're spending tons of time in and under nsDefaultComparator<mozilla::HandleRefPtr<mozilla::StyleSheetHandle>,mozilla::HandleRefPtr<mozilla::StyleSheetHandle>>
If we can trust the profile, looks like the Equals is some super slow, doing addrefs and what not.

(I have no idea what HandleRefPtr is and why normal RefPtr wouldn't work)

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Is this a regression from bug 1244074. I'm pretty sure this is, or at least that added some of the slow paths here.

Comment 13

[Cameron McCormack (:heycam)]

(In reply to Olli Pettay [:smaug] (vacation Aug 25-28) from comment #11)
> We're spending tons of time in and under
> nsDefaultComparator<mozilla::HandleRefPtr<mozilla::StyleSheetHandle>,mozilla:
> :HandleRefPtr<mozilla::StyleSheetHandle>>
> If we can trust the profile, looks like the Equals is some super slow, doing
> addrefs and what not.

Ouch. :(

nsDefaultComparator<
  mozilla::HandleRefPtr<mozilla::StyleSheetHandle>,
  mozilla::StyleSheetHandle
>::Equals(mozilla::HandleRefPtr<mozilla::StyleSheetHandle> const&,
          mozilla::StyleSheetHandle const&)

It's because we're searching for a raw pointer in an array of strong pointers, and HandleRefPtr doesn't have an operator== that works with that.  So instead we're ending up constructing a strong reference to do the comparison...

> (I have no idea what HandleRefPtr is and why normal RefPtr wouldn't work)

The Handle objects are not pointers, and RefPtr only works with pointer typed things.

Comment 14

[Cameron McCormack (:heycam)]

Attachment: Add operator== overloads for comparing HandlRefPtrs to their raw Handles.

Michael, would you mind profiling with this patch applied, to confirm this helps?  I tested locally to verify nsTArray<HandleRefPtr<StyleSheetHandle>>::IndexOf(StyleSheetHandle) calls this new operator==.

Comment 15

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8a95afd5f72

Comment 16

[Bobby Holley (:bholley)]

Comment on attachment 8784717 [details] [diff] [review]
Add operator== overloads for comparing HandlRefPtrs to their raw Handles.

Review of attachment 8784717 [details] [diff] [review]:
-----------------------------------------------------------------

I feel like there's probably some clever template magic we can do here, but these handles aren't long for this earth anyway. Thanks for fixing.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/906c1f635c43
Add operator== overloads for comparing HandlRefPtrs to their raw Handles. r=bholley

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/906c1f635c43

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Since at least the part which was fixed is a regression, should we take the patch to branches?

Comment 20

[Nika Layzell [:nika] (ni? for response)]

I pulled & built central, and have been leaving it sitting on an article for the last little bit. The lag is definitely improved from before, but there is still some scrolling jank. I believe that this jank is present in chrome too, so it might be our fault, but once the cleopatra servers are back up I'll try to post a profile.

Comment 21

[Cameron McCormack (:heycam)]

Thanks, Michael.  Olli, yes I think we should take this on branches.

Comment 22

[Cameron McCormack (:heycam)]

Comment on attachment 8784717 [details] [diff] [review]
Add operator== overloads for comparing HandlRefPtrs to their raw Handles.

Approval Request Comment
[Feature/regressing bug #]: bug 1244074
[User impact if declined]: performance regression on sites that add many style sheets
[Describe test coverage new/current, TreeHerder]: locally tested to verify we no longer unnecessarily AddRef/Release style sheets in this case
[Risks and why]: low -- this is just changing the comparison function used by nsTArray searching functions
[String/UUID change made/needed]: N/A

Comment 23

[Cameron McCormack (:heycam)]

[Tracking Requested - why for this release]: performance regression going back to Firefox 47

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8784717 [details] [diff] [review]
Add operator== overloads for comparing HandlRefPtrs to their raw Handles.

Fixing a recent(ish) perf regression, Aurora50+

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/aaa878146a67

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8784717 [details] [diff] [review]
Add operator== overloads for comparing HandlRefPtrs to their raw Handles.

Nice detective work! Let's bring this fix to beta 9. It is late in the cycle but we can try verifying the fix on Friday on beta.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei, can your team test once this lands?  Abe, also fine for you to test if you get to it sooner - once it lands on mozilla-beta you can get it from treeherder. Because of the timing of when it lands, I'm not sure who will see it first :)

Comment 28

[Abe - QA (:Abe_LV)]

If it lands in PST working hours, I will get it first. Otherwise Andrei will have it.

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/057ddf18f231

Comment 30

[Abe - QA (:Abe_LV)]

We have tested this issue on windows XP with the latest build from treeherder [https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&selectedJob=1566414] 

In addition to reproducing the existing issue, we also observed other two new issues using this build.

Here are our observations:
1.It is still slow and shows the unresponsive script warning dialog (existing issue)
2.Lots of cascades in left and right side of the page (new issue)
3.Unable to resume to live update when “Resume live updates” button is clicked (new issue)

Bugs will be filed for these new issues. 

Here is screen capture: https://testing-1.tinytake.com/sf/OTQ4Nzc3XzM5ODEwMjg

---
Version 		49.0b9
Build ID 		20160831145122
Update Channel 	beta
User Agent 	Mozilla/5.0 (Windows NT 5.1; rv:49.0) Gecko/20100101 Firefox/49.0

Comment 31

[Liz Henry (:lizzard) (relman/hg->git project)]

heycam, can you take a look? Should we back this out in 49 since it seems to have caused new regressions? We can try again in 50 to come up with a fix.

Comment 32

[Liz Henry (:lizzard) (relman/hg->git project)]

Sounds like the regressions in bug 1299908 were caused by work from bug 1296793, not from this perf fix. 
The slow script issue may be the same as in bug 1284511.

Comment 33

[Andrei Vaida [:avaida]]

Seems Abe beat me to it. Liz, please ni? me if my team should further look into this. 

Note that it's more difficult for us to reproduce this bug, because there are close to no new comments displayed while testing washingtonpost.com in our timezone.

Comment 34

[q1]

The behavior described by this bug still exists in FF 49.0.1. The steps to reproduce are exactly the same. Should I re-open this bug? File another?

Comment 35

[Ryan VanderMeulen [:RyanVM]]

(In reply to q1 from comment #34)
> The behavior described by this bug still exists in FF 49.0.1. The steps to
> reproduce are exactly the same. Should I re-open this bug? File another?

At this point, yes. Sorry for the hassle and long lag!

Comment 36

[q1]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #35)
> (In reply to q1 from comment #34)
> > The behavior described by this bug still exists in FF 49.0.1. The steps to
> > reproduce are exactly the same. Should I re-open this bug? File another?
> 
> At this point, yes. Sorry for the hassle and long lag!

OK, I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1313452 awhile back.