Closed Bug 958802 Opened 11 years ago Closed 11 years ago

Speed up mochitest test_extra_inherit_initial.html

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files, 2 obsolete files)

bug 958075 comment 7 has some ideas for speeding up the mochitest test_extra_inherit_initial.html (which currently takes over 300 seconds on B2G opt, and over 900 seconds on B2G debug). I'm spinning off this bug to cover those speedups.
OS: Linux → All
Hardware: x86_64 → All
Attached patch part 1: process properties in larger batches (obsolete) (deleted) — Splinter Review
This patch us only yield after processing 20 properties (instead of after every single property).
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8358767 - Flags: review?(bzbarsky)
Attachment #8358768 - Flags: review?(bzbarsky)
Comment on attachment 8358767 [details] [diff] [review] part 1: process properties in larger batches Are you sure this is safe? I saw some single properties take multiple seconds in the b2g test log you linked me to (e.g. "animation", which has tons of subproperties, with tons of values), and the default slow script dialog is after 10s...
Comment on attachment 8358768 [details] [diff] [review] part 2: turn off CSS error reporting for this test r=me, but how much does it help the b2g times?
Attachment #8358768 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #4) > r=me, but how much does it help the b2g times? Quite a lot -- see bug 958075 comment 9 & through 11. tl;dr: - batching (part 1 here) improved our time by 9% of the time in the first trial, by 36% in the second trial. - disabling error reporting (part 2 here) improved our time by 34% in the first trial, 30% in the second. - Both together saved ~42. (only did one trial of that) I'll take a closer look at batching to see if we might get close to triggering the slow script dialog. I'm surprised I didn't trigger it in my Try run, given your comment 3. Maybe the slow script dialog is disabled on B2G mochitests?
Hmm. I'd been hoping for more speedup, but maybe the emulator really is just that slow. :(
Comment on attachment 8358767 [details] [diff] [review] part 1: process properties in larger batches r=me assuming this works.
Attachment #8358767 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #6) > Hmm. I'd been hoping for more speedup, but maybe the emulator really is > just that slow. :( Yeah. Gregor said in IRC that it's emulating ARM on x86, and we haven't really invested in optimizing the emulator yet, so it's known to be much slower than everything else.
(In reply to Daniel Holbert [:dholbert] from comment #5) > I'll take a closer look at batching to see if we might get close to > triggering the slow script dialog. I'm surprised I didn't trigger it in my > Try run, given your comment 3. Maybe the slow script dialog is disabled on > B2G mochitests? Actually, from my testing, it looks like the slow script dialog is disabled altogether for mochitests. So I don't think we need batching at all. I'll post a new patch that just drops the batching. More details: I tried doing everything in one batch here (no yielding), and got no issues. Then, for good measure, I tried doing each property 3 times (to triple the test duration, so that we'd be significantly longer than 30 seconds on Desktop platforms), and I still got no issues. (aside from *Output exceeded 52428800 bytes, remaining output has been truncated, but that was after this test completed.) I suspect maybe the slow script dialog might have been a problem back when dbaron initially wrote this test in 2009, but it's apparently not a problem for mochitests now.
Try run with all the properties in one single batch: https://tbpl.mozilla.org/?tree=Try&rev=e5392f58be46 Shows no slow-script / timeout issues (with this test) on any platform. Posting updated version of patch #1 shortly.
Attachment #8358767 - Attachment is obsolete: true
Attachment #8360179 - Flags: review?(bzbarsky)
Comment on attachment 8360179 [details] [diff] [review] part 1, v2: process all properties without yielding r=me
Attachment #8360179 - Flags: review?(bzbarsky) → review+
Attachment #8358768 - Attachment is obsolete: true
Attachment #8360181 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: