Closed Bug 1088771 Opened 10 years ago Closed 10 years ago

CSS variable usage within browser.css on MacOSX 10.8 causes a 2.12% increase in Customization Animation Tests

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: bgrins, Unassigned)

References

Details

(Keywords: perf, regression, Whiteboard: [talos_regression])

More information in the message below. Regression: Fx-Team - Customization Animation Tests - MacOSX 10.8 - 2.63% increase ---------------------------------------------------------------------------------- Previous: avg 35.577 stddev 0.307 of 12 runs up to revision 3df00a905527 New : avg 36.513 stddev 0.395 of 12 runs since revision a890f2283b74 Change : +0.936 (2.63% / z=3.044) Graph : http://mzl.la/1tsq7iJ Changeset range: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=3df00a905527&tochange=a890f2283b74 Changesets: * http://hg.mozilla.org/integration/fx-team/rev/a890f2283b74 : Brian Grinstead <bgrinstead@mozilla.com> - Bug 1053434 - Switch some hardcoded OSX values to CSS variables for devedition;r=Gijs : http://bugzilla.mozilla.org/show_bug.cgi?id=1053434 Bugs: * http://bugzilla.mozilla.org/show_bug.cgi?id=1053434
Cameron, I'm not sure how to tackle this, any ideas? I have similar patches in review right now for windows and linux that I was planning to land within the next week.
Flags: needinfo?(cam)
Blocks: 1053434
Keywords: perf, regression
Whiteboard: [talos_regression]
I don't know how to read this data, is the problem still persisting after a few days of new tests? If so, will we need to back out the change assuming that the variable performance can't be optimized in time for the release?
Flags: needinfo?(jmaher)
Summary: CSS variable usage within browser.css on MacOSX 10.8 causes a 2.63% increase in Customization Animation Tests → CSS variable usage within browser.css on MacOSX 10.8 causes a 2.12% increase in Customization Animation Tests
(In reply to Brian Grinstead [:bgrins] from comment #4) > I don't know how to read this data, is the problem still persisting after a > few days of new tests? You can click the graph link in comment #0 and switch to 30 days, and you can see the increase (however small it is). It doesn't seem to have magically gone away yet, no. > If so, will we need to back out the change assuming > that the variable performance can't be optimized in time for the release? Unsure. 2.1% on this particular test isn't as bad as it could be. I'd be worried if tpaint or ts_paint is also affected. I also don't really understand /why/ this particular test is affected - I'd expect tpaint/ts_paint to suffer more. I also don't know what our perf optimization range is, ie how easy it is to fix the performance here. I'm also interested as to whether the effects on Windows and Linux are going to be as bad...
Summary from email: Jet: > I'm seeing a lot of noise in the test graph: > http://graphs.mozilla.org/graph.html#tests=[[309,64,24]]&sel=1413966342034.9492,1414133284407.8306&displayrange=7&datatype=running > > ie. we go up and down to the perf levels before the CSS vars were added. > > I'd expect some overhead from CSS vars, and this change looks like it's > within the noise threshold. Are you guys seeing other perf test regressions? Brian: > This is the only regression that I've been notified of. > According to https://bugzilla.mozilla.org/show_bug.cgi?id=1088771#c5 > this particular test is less worrisome than if it were tpaint or tspaint. > It's sounding like we should proceed with landing the windows/linux > variations of this patch and see if there is a similar regressions. > > If so, do you think we would be OK for these to stick and even be uplifted > to Aurora? Jet: > I think so, but please keep the perf bug open as we'd still like to see if > Cam finds a fix for us when he gets back in November. Drop me a line if > you see other fallout from this change, and we can run another profile.
looking at other platforms: http://graphs.mozilla.org/graph.html#tests=%5B%5B309,64,24%5D,%5B309,132,33%5D,%5B309,132,25%5D,%5B309,132,35%5D%5D&sel=1413898496247,1414503296247&displayrange=7&datatype=running we see win7, linux32 and linux64 to have problems as well. The win7 regression seems so small though. Note, win8 had a hiccup with results due to a 32 vs 64 and pgo vs nonpgo switch, so I am ignoring any win8 regressions until next week. As it stands, this appears to be on trunk builds only, not on Aurora (unless I am mistaken somehow). Even a small fix here would be helpful as we have a series of small cart regression in v.36.
Flags: needinfo?(jmaher)
(In reply to Joel Maher (:jmaher) from comment #7) > As it stands, this appears to be on trunk builds only, not on Aurora (unless > I am mistaken somehow). To be clear, these are not already on Aurora - we are going to want to uplift these patches to Aurora soon, most likely this week.
Blocks: 1091001
Cam, just curious if you may have a chance to look at this? I have another patch I'd like to land that converts more things to CSS variables for Bug 1091001, but I'm seeing what looks like a cart regression on the try push: http://compare-talos.mattn.ca/dev/?oldBranch=Fx-Team&oldRevs=94be697bd801&newBranch=Try&newRev=a156536eadd1&submit=true
I do not see this on mozilla-beta, can we close this?
(In reply to Joel Maher (:jmaher) from comment #11) > I do not see this on mozilla-beta, can we close this? I've still bumped into regressions when trying to land a patch that converts more values to variables - see Bug 1091001 Comment 19 and Comment 20. I ended up just undoing some of the changes until the regression wasn't showing up on a try push. If this is no problem on beta though, I'd say we can close it. I can open a new bug to deal with those specific performance issues I was seeing.
(In reply to Brian Grinstead [:bgrins] from comment #12) > If this is no problem on beta though, I'd say we can close it. I can open a > new bug to deal with those specific performance issues I was seeing. Filed Bug 1122726. I will re-run talos with that patch and ni? Cam if I'm still seeing regressions there.
Flags: needinfo?(cam)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.