Closed Bug 831123 Opened 8 years ago Closed 8 years ago

Tp regression on Jan 11th

Categories

(Firefox for Android Graveyard :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 21

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file, 2 obsolete files)

I think this might be coming from:
http://hg.mozilla.org/integration/mozilla-inbound/rev/028ce4b36b4e

Only because we send error output to the Android logcat, which has a measurable affect on performance. I made some Try runs with the CSS error reporting turned off and found Tp was faster:
https://tbpl.mozilla.org/?tree=Try&rev=3df9fa8c6692

Since there is no easy way to see the errors, I suggest we disable outputting CSS errors until we add a dev UI for toggling them on/off.
Attached patch patch (obsolete) — Splinter Review
Disable CSS error reporting by default
Assignee: nobody → mark.finkle
Attachment #703138 - Flags: review?(blassey.bugs)
Attachment #703138 - Flags: review?(blassey.bugs) → review+
backed out for test failures... not pasting them cause "Only displaying first 20 of 100+ failures"
https://tbpl.mozilla.org/php/getParsedLog.php?id=18896131&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7ae1bf4479a
Attached patch patch 2 (obsolete) — Splinter Review
Same as previous patch, but adds fixes to the two tests that failed. I use SpecialPowers.pushPrefEnv to temporarily set the pref for the test. Try run is green now:
https://tbpl.mozilla.org/?tree=Try&rev=697b280e4c0b
Attachment #703138 - Attachment is obsolete: true
Attachment #703627 - Flags: review?(jmaher)
Comment on attachment 703627 [details] [diff] [review]
patch 2

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

looks great, thanks for using these specialpowers functions.
Attachment #703627 - Flags: review?(jmaher) → review+
Attached patch alt patchSplinter Review
I was not able to figure out a way to fix test_bug513194.html:
http://mxr.mozilla.org/mozilla-central/source/content/base/test/test_bug513194.html?force=1

So, I did a quickie fix and turned on the pref for mochitests using the pre-seed file. The pref is not pre-seeded for talos, so we can measure the impact on Tp4.
Attachment #703627 - Attachment is obsolete: true
Attachment #704601 - Flags: review?(jmaher)
Try Server run with green M1 and M8 runs. Tp4 is improved.
(In reply to Mark Finkle (:mfinkle) from comment #8)
> Try Server run with green M1 and M8 runs. Tp4 is improved.

The link would be handy:
https://tbpl.mozilla.org/?tree=Try&rev=86b551105152
Comment on attachment 704601 [details] [diff] [review]
alt patch

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

r=me if we can get a bug on file to fix the two tests needed to not hack this setting on.
Attachment #704601 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/f8cb1716f5d4
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.