Closed Bug 1157648 Opened 5 years ago Closed 5 years ago

Opening Pinterest on FxOS is much slower than Android (via WiFi), due to CSS error reporting

Categories

(Core :: CSS Parsing and Computation, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla40
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 --- wontfix
firefox40 --- fixed
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: ting, Assigned: dbaron)

References

()

Details

Attachments

(1 file)

Video [1] 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) [2]

Open the same page by Chrome on Android Nexus4 with image 5.1.0 (LMY47O) [3].

[1] http://youtu.be/TOiYS2mio8A
[2] https://dl.google.com/dl/android/aosp/hammerhead-lmy47i-factory-df127988.tgz
[3] https://dl.google.com/dl/android/aosp/occam-lmy47o-factory-cae68e81.tgz
Profile [1] shows a 12 seconds long Loader::ParseSheet()...

[1] 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.
No longer blocks: 1094010
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+
https://hg.mozilla.org/mozilla-central/rev/b2dd209a43df
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Ting-Yu, should we try to uplift this to at least v2.2? It does not seem risky at all.
Flags: needinfo?(janus926)
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
Flags: needinfo?(dbaron)
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+
Keywords: verifyme
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.