Closed Bug 1157648 Opened 5 years ago Closed 5 years ago
Opening Pinterest on Fx
OS is much slower than Android (via Wi Fi), due to CSS error reporting
Video  of opening https://www.pinterest.com/categories/everything via WiFi. FxOS runs on a Nexus5 with versions: Gaia: a7dcc5fb595030dab140d5ff0e7eb5ef04017d51 Gecko: https://hg.mozilla.org/mozilla-central/rev/946ac85af8f4 Image: 5.1.0 (LMY47I)  Open the same page by Chrome on Android Nexus4 with image 5.1.0 (LMY47O) .  http://youtu.be/TOiYS2mio8A  https://dl.google.com/dl/android/aosp/hammerhead-lmy47i-factory-df127988.tgz  https://dl.google.com/dl/android/aosp/occam-lmy47o-factory-cae68e81.tgz
Profile  shows a 12 seconds long Loader::ParseSheet()...  http://people.mozilla.org/~bgirard/cleopatra/#report=51229b511d3483991e70895df3db0b5ad889a1f3
Component: Performance → CSS Parsing and Computation
Product: Firefox OS → Core
Version: unspecified → Trunk
Mostly error reporting. Probably either a very long line or a very long URL. Does it still happen with RELEASE_BUILD defined (see the ifdefs in nsConsoleService::LogMessageWithMode)?
Summary: Opening Pinterest on FxOS is much slower than Android (via WiFi) → Opening Pinterest on FxOS is much slower than Android (via WiFi), due to CSS error reporting
Yes, now it's almost as fast as Android.
This means that when mSourceName and mSourceLine are large, ToString is not excessively expensive. This is particularly important for CSS errors, where we don't make an attempt to truncate these prior to constructing the script error, but we do ensure that when we report multiple errors on the same line (which is common for minified CSS), we share from the same string buffer and avoid copying.
Attachment #8598053 - Flags: review?(bobbyholley)
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #8598053 - Flags: review?(bobbyholley) → review+
Ting-Yu, should we try to uplift this to at least v2.2? It does not seem risky at all.
Sure. dbaron, if there're no other concerns, could you please uplift this to v2.2 (set flag approval‑mozilla‑b2g37)? Thanks.
Flags: needinfo?(janus926) → needinfo?(dbaron)
Comment on attachment 8598053 [details] [diff] [review] Make nsScriptError::ToString use only the first 512 characters of mSourceName and mSourceLine NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): not sure User impact if declined: slow parsing of minified CSS with large numbers of errors Testing completed: fixes problem; in nightly for a few days Risk to taking this patch (and alternatives if risky): low risk; the only risk would be that something is depending on the string representation of errors having more of the filename or line, but that seems pretty unlikely String or UUID changes made by this patch: none
Attachment #8598053 - Flags: approval-mozilla-b2g37?
Comment on attachment 8598053 [details] [diff] [review] Make nsScriptError::ToString use only the first 512 characters of mSourceName and mSourceLine I am approving this although not a blocker as this seems like a good perf improvement and since 2.2 might be slightly long lived. Having said that, if this results fallouts that block the release, we should back this out than doing additional forward fixing.
Attachment #8598053 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
This issue is verified fixed on Flame 2.5, Flame 2.2, and Aries. The mentioned Pinterest page takes ~3 seconds to *display on Aries, ~12 seconds to display on Flame 2.5 (before it takes ~23 seconds), ~8 seconds to display on Flame 2.2. *Definition of 'display': Webpage graphics are displayed, but loading bar continues to load and has not finished loading everything. Device: Flame 2.2 (319MB memory) BuildID: 20150914032507 Gaia: 7a427e0f8aa6c185a9e22358006b97c19435ca4a Gecko: 0d9c46d01861 Gonk: bd9cb3af2a0354577a6903917bc826489050b40d Version: 37.0 (2.2) Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Device: Flame 2.5 (319MB memory) BuildID: 20150914030233 Gaia: 4d9b996be4b1935651057d0651461c1a36d98a18 Gecko: 9ed17db42e3e46f1c712e4dffd62d54e915e0fac Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd Version: 43.0a1 (2.5) Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0 Device: Aries 2.5 BuildID: 20150914130903 Gaia: f37e8f732e0af961b43e912629c84c9e2ceda55d Gecko: fba4b0cd3823975949765acc0b16b964d1712b75 Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd Version: 43.0a1 (2.5) Firmware Version: D5803_23.1.A.1.28_NCB.ftf User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
You need to log in before you can comment on or make changes to this bug.