Poor performance when viewing washingtonpost.com comments

RESOLVED FIXED in Firefox 49



CSS Parsing and Computation
a year ago
11 months ago


(Reporter: q1, Assigned: heycam)



47 Branch
Windows XP

Firefox Tracking Flags

(firefox48- wontfix, firefox49+ fixed, firefox50+ fixed, firefox51 fixed)




(2 attachments)



a year ago
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).


a year ago
Version: 46 Branch → 47 Branch

Comment 1

a year ago
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? 

Flags: needinfo?(q1)

Comment 2

a year ago
(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.
Name 	Version 	Enabled 	ID
Adblock Plus	2.7.3	true	{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}
Firefox Hello	1.4.3	true	loop@mozilla.org
Multi-process staged rollout	1.0	true	e10srollout@mozilla.org
Pocket	1.0.4	true	firefox@getpocket.com
Microsoft .NET Framework Assistant	0.0.0	false	{20a82645-c095-46ed-80e3-08825760534b}
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 (
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
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.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.used	true
dom.push.connection.enabled	false
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.storage.version.observed	1
media.hardware-video-decoding.failed	true
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
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_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_footerleft	&PT
print.printer_HP_LaserJet_2300_Series_PCL_6.print_footerright	&D
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_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_resolution	600
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_footerleft	&PT
print.printer_Microsoft_XPS_Document_Writer.print_footerright	&D
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_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_footerleft	&PT
print.printer_Samsung_M2070_Series.print_footerright	&D
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_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
security.OCSP.enabled	0
storage.vacuum.last.index	1
storage.vacuum.last.places.sqlite	1469030338
Important Locked Preferences
	Name 	Value
Incremental GC 	true
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
Flags: needinfo?(q1)

Comment 3

a year ago
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

a year ago
Created attachment 8782502 [details]

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


a year ago
Component: Untriaged → DOM
Product: Firefox → Core
Can one of you that can reproduce please get a profile? https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Reporting_a_Performance_Problem
Flags: needinfo?(q1)
Flags: needinfo?(amasresha)

Comment 6

a year ago
(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.
Flags: needinfo?(q1)

Comment 7

a year ago
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

a year ago
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

a year ago
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
Flags: needinfo?(amasresha)
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:

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.
Flags: needinfo?(bugs)
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)
Component: DOM → CSS Parsing and Computation
Flags: needinfo?(bugs) → needinfo?(cam)
Is this a regression from bug 1244074. I'm pretty sure this is, or at least that added some of the slow paths here.
Blocks: 1244074

Comment 13

a year ago
(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. :(

>::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.
Flags: needinfo?(cam)


a year ago
Assignee: nobody → cam

Comment 14

a year ago
Created attachment 8784717 [details] [diff] [review]
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==.
Flags: needinfo?(michael)
Attachment #8784717 - Flags: review?(bobbyholley)


a year ago
Attachment #8784717 - Attachment is patch: true

Comment 15

a year ago
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.
Attachment #8784717 - Flags: review?(bobbyholley) → review+

Comment 17

a year ago
Pushed by cmccormack@mozilla.com:
Add operator== overloads for comparing HandlRefPtrs to their raw Handles. r=bholley
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Since at least the part which was fixed is a regression, should we take the patch to branches?
Flags: needinfo?(cam)
Keywords: regression
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.
Flags: needinfo?(michael)

Comment 21

a year ago
Thanks, Michael.  Olli, yes I think we should take this on branches.
Flags: needinfo?(cam)

Comment 22

a year ago
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
Attachment #8784717 - Flags: approval-mozilla-beta?
Attachment #8784717 - Flags: approval-mozilla-aurora?

Comment 23

a year ago
[Tracking Requested - why for this release]: performance regression going back to Firefox 47
tracking-firefox48: --- → ?
tracking-firefox49: --- → ?
tracking-firefox50: --- → ?


a year ago
status-firefox49: --- → affected
status-firefox50: --- → affected


a year ago
status-firefox48: --- → affected
Comment on attachment 8784717 [details] [diff] [review]
Add operator== overloads for comparing HandlRefPtrs to their raw Handles.

Fixing a recent(ish) perf regression, Aurora50+
Attachment #8784717 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+


a year ago
tracking-firefox50: ? → +
status-firefox48: affected → wontfix
tracking-firefox48: ? → -

Comment 25

a year ago
status-firefox50: affected → fixed
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.
Attachment #8784717 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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 :)
Flags: needinfo?(andrei.vaida)
Flags: needinfo?(amasresha)
If it lands in PST working hours, I will get it first. Otherwise Andrei will have it.
Flags: needinfo?(amasresha)

Comment 29

a year ago
status-firefox49: affected → fixed
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
Flags: needinfo?(lhenry)


a year ago
See Also: → bug 1299908
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.
Flags: needinfo?(lhenry) → needinfo?(cam)
tracking-firefox49: ? → +
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.
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.
Flags: needinfo?(andrei.vaida)

Comment 34

a year ago
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?
(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!
Flags: needinfo?(cam)

Comment 36

11 months ago
(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.
You need to log in before you can comment on or make changes to this bug.