Closed Bug 1230280 Opened 9 years ago Closed 8 years ago

2% Win7 e10s ts_paint, win8 paint regression on fx.team (v.45) on Nov 27, 2015 from push 19876a153a00

Categories

(Hello (Loop) :: Client, defect, P2)

defect

Tracking

(e10s-)

RESOLVED INVALID
Tracking Status
e10s - ---

People

(Reporter: jmaher, Assigned: mikedeboer)

References

Details

(Keywords: perf, regression, Whiteboard: [talos_regression][btpp-followup-2016-04-25])

Attachments

(1 file)

A small ts_paint regression exists from some patches in bug 1223573 which landed on Nov 27th on fx-team. After some retriggers: https://treeherder.mozilla.org/#/jobs?repo=fx-team&filter-searchStr=talos%20e10s%20other%20win&fromchange=556ab9e3521b&tochange=d7152cc38e8f we can see a compareview: https://treeherder.mozilla.org/perf.html#/compare?originalProject=fx-team&originalRevision=f48352b311eb&newProject=fx-team&newRevision=19876a153a00&filter=ts_paint&showOnlyConfident=1 these are small regressions and from what I can tell the only real regressions coming from this specific push are the ts_paint ones. These are small, but seem to be real (despite the large volume of noise- shifting patterns of noise!)
Mark, sorry to bother you on so many bugs- finally catching up on all the alerts that have been coming in. This is small- but we should see if there is something simple to fix this vs ignore it completely. Maybe this is expected and we can have it documented here in this bug.
Flags: needinfo?(standard8)
Ok, so I'm guessing this is a similar issue to the size bug - there's extra initialisation going on for the add-on. We're also registering style sheets that we weren't before. I'm not sure which would be the biggest impact - the add-on registration or the style sheets, but I would guess it could be either of them, and I doubt there's much we could do here.
Flags: needinfo?(standard8)
is there a quick check we could do to see if it is the registration or the style sheets? The more we understand the better we can make decisions in the future even if there is nothing to fix for this specific issue.
You could probably just comment it out in the code: http://hg.mozilla.org/mozilla-central/annotate/5ba77225c957/browser/extensions/loop/bootstrap.js#l800 Perhaps not fully the same, but it might be close enough.
ok, commenting out the style sheet code: https://hg.mozilla.org/try/rev/de7d9e9896c5 didn't do the trick according to compare chooser: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=53859be1a511&newProject=try&newRevision=3155506346cc&showOnlyImportant=0 We have pretty much identical results for the baseline and the commented out version.
Mark, are you tracking this?
Flags: needinfo?(standard8)
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #7) > Mark, are you tracking this? Roughly. Unfortunately I have very little experience of these perf tests so I've been relying on Joel. I'm also not sure there is much that we can do about it - but that depends on where the issue is. Taking out the style sheets was my best guess. I guess we could try taking out most of bootstrap.js and see if anything changes. Also cc'ing other folks who might have experience of this area.
Flags: needinfo?(standard8)
I'll see if I can tease a profile out of this for you.
Flags: needinfo?(mconley)
Summary: 2% Win7 e10s ts_paint regression on fx.team (v.45) on Nov 27, 2015 from push 19876a153a00 → 2% Win7 e10s ts_paint, win8 paint regression on fx.team (v.45) on Nov 27, 2015 from push 19876a153a00
Rank: 19
Priority: -- → P1
I'll be driving this bug. Mike, what kind of profile were you thinking about?
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Bah - sorry I never got to this - holidays happened. Pushed for profiling to try: Without bug 1223573 ("good"): https://treeherder.mozilla.org/#/jobs?repo=try&revision=33a9120cc7a9 With bug 1223573 ("bad"): https://treeherder.mozilla.org/#/jobs?repo=try&revision=57eadf5209e7
Flags: needinfo?(mconley)
Bah - and resetting the needinfo on myself to do the analysis once these come in.
Flags: needinfo?(mconley)
This is not e10s only (it seems to affect Win8 64, Linux 64, and Windows XP, all non-e10s), so minusing.
tracking-e10s: --- → -
Rank: 19 → 25
Priority: P1 → P2
Rank: 25 → 19
Priority: P2 → P1
not sure what the two pushes are on try, but ts_paint isn't looking as a regression.
(In reply to Joel Maher (:jmaher) from comment #18) > not sure what the two pushes are on try, but ts_paint isn't looking as a > regression. The newRevision commit contains the 'fix' I wanted to test out if it'd resolve the regression. It doesn't, unfortunately.
Rank: 19 → 25
Priority: P1 → P2
If the theory is that this is invoking the CustomizableUI that's at issue, was a similar regression seen when the pocket add-on landed?
Mike, here's a thought: Pocket landed after we did, but uses the same startup flow that we do - create the widget when the add-on's startup() function is called. Your try push only moved the creating for the widget for Hello, not for Pocket. Maybe we need to fix both add-ons to fix the regression? In the event this does fix it, maybe customisable UI could be set to warn/reject if a createWidget or other call comes in before the expected startup?
Flags: needinfo?(mdeboer)
Could be true - not sure unless we try!
Flags: needinfo?(mdeboer)
Status: ASSIGNED → NEW
Ed, its possibly getting a bit late for this bug now, but could you take on doing a fix as per my comment 21? I think Mike can help with which try pushes to do / how to get perf info.
Flags: needinfo?(edilee)
Ed, also ping me or ask questions in the bug, running on try is fairly easy and viewing the results are not too difficult once you know where to look.
Baseline ts_paint: https://hg.mozilla.org/try/rev/f52121663cedf7a7bc3ca2d53672098f7522e4d1 https://treeherder.mozilla.org/#/jobs?repo=try&revision=f52121663ced talos-linux64-ix: 1173, 1180, 1179, 1173, 1187 -> 1,178 average tst-linux64-spot: 2747, 2478, 2802, 2627, 2626 -> 2,656 average browser-ui-startup-complete ts_paint: https://hg.mozilla.org/try/rev/44bf96fe66adee013f1417e90e6b4e49a64b94fb https://treeherder.mozilla.org/#/jobs?repo=try&revision=44bf96fe66ad talos-linux64-ix: 1180, 1185, 1170, 1183, 1182 -> 1,180 average tst-linux64-spot: 2715, 2760, 2505, 2272, 2651 -> 2,581 average Not much change for the first set of machines but roughly 2% for the tst-* machines (although the variance is much higher). https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=f52121663ced&newProject=try&newRevision=44bf96fe66ad&framework=7&filter=ts_paint&showOnlyImportant=0
Flags: needinfo?(edilee)
(In reply to Ed Lee :Mardak from comment #25) > Not much change for the first set of machines but roughly 2% for the tst-* > machines (although the variance is much higher). Please just ignore the tst-* results, that's an experiment gone wrong. See bug 1260926.
What's the way forward here now? Or do we close this as won't/can't fix?
Flags: needinfo?(mdeboer)
Flags: needinfo?(mconley)
Flags: needinfo?(jmaher)
Whiteboard: [talos_regression] → [talos_regression][btpp-followup-2016-04-25]
If it's any consolation, I have a fix for a completely unrelated bug that looks like it might give us a nice ts_paint win across the board (bug 1261842), so I don't know how much we should prioritize this investigation, tbh.
Flags: needinfo?(mconley)
Flags: needinfo?(mdeboer)
hello removed so we can close this bug
(In reply to timse201 from comment #30) > hello removed so we can close this bug Mark, OOI, was there a perf win when Hello got removed again?
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(standard8)
Resolution: --- → INVALID
Yes ts_paint did improve, although cart regressed for some reason. See bug 1290808.
Depends on: 1290808
Flags: needinfo?(standard8)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: