Talos has detected a Firefox performance regression from push: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=80a0c38278154ddf519ee8ff9e7239e938540b37 As author of one of the patches included in that push, we need your help to address this regression. Regressions: 11% tpaint summary windows10-64 pgo e10s 218.04 -> 241.38 9% tpaint summary windows10-64 opt e10s 275.73 -> 301.12 You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=8996 On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format. To learn more about the regressing test(s), please see: https://wiki.mozilla.org/Buildbot/Talos/Tests For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/Talos/Running *** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! *** Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Buildbot/Talos/RegressionBugsHandling
Component: Untriaged → DOM: Security
Product: Firefox → Core
Christoph, I see you are the owner of bug 1393350. Looks like this is something we should accept, but I still want to know if you find something strange about these regressions. If not, I'll resolve this to WONTFIX.
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #1) > Christoph, I see you are the owner of bug 1393350. Looks like this is > something we should accept, but I still want to know if you find something > strange about these regressions. If not, I'll resolve this to WONTFIX. We didn't modify the test itself, we just load what was formerly a data: URI from an external file now to comply with the new data: URI inheritance model. I can obviously try to help get any issues resolved, but I can't tell if those are real regressions and if we have to resolve them. Jmaher is probably the better person to ask, I'll change the ni? but will continue to watch this bug.
Flags: needinfo?(ckerschb) → needinfo?(jmaher)
I am curious why this is tpaint only and why this is windows 10 only? is there something about the data:URI changes that affect windows specifically or 64 bit only? if we can understand why this is happening for win10 only that would make me happier to say "lets accept this as a test change"
Flags: needinfo?(jmaher) → needinfo?(ckerschb)
oh, 34% regression on linux64, possibly this is just a 64 bit issue?
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #3) > I am curious why this is tpaint only and why this is windows 10 only? is > there something about the data:URI changes that affect windows specifically > or 64 bit only? There is nothing platform specific for the data: URI changes we performed. But 34% regression on Linux really sounds like a lot. In particular, this change only loads html from an external file instead of using an inline data: URI. I would imagine it's rather something in the test itself than really a performance regression caused by the Gecko changes. Possibly this is the new baseline? Alternatively we could rewrite the test yet again, go back to using a data: URI an just replace |window.opener.childIsOpen()| with some postMessge handler to bypass the origin restrictions. Do you want me to try that?
Flags: needinfo?(ckerschb) → needinfo?(jmaher)
Loading a file takes of course more time than data: url which is all in memory from the beginning. I wonder if the test could be changed to use data: again but with some postMessages or so?
I don't mind the change in the test values as a new baseline, I would just rather we have a better explanation for it.
Created attachment 8901709 [details] [diff] [review] bug_1393742_tpaint_postmessage.patch Let's check if using a postmessage handler changes things here again.
Attachment #8901709 - Flags: review?(bugs)
Attachment #8901709 - Flags: review?(bugs) → review+
I am marking this one as a 'leave-open' so we can have a look at perf numbers with that change applied.
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Keywords: checkin-needed, leave-open
Priority: -- → P3
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6d8af9855103 Rewrite test tpaint.html to rely on data: URI and postmessage handler to avoit perf regression. r=smaug
I see perf improvements: == Change summary for alert #9077 (as of August 28 2017 13:48 UTC) == Improvements: 26% tpaint summary linux64 pgo e10s 285.39 -> 209.93 26% tpaint summary linux64 opt e10s 315.17 -> 234.39 20% tpaint summary osx-10-10 opt e10s 418.62 -> 335.70 8% tpaint summary windows10-64 opt e10s304.73 -> 281.42 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9077
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #12) > I see perf improvements: That looks way better. Are you fine with marking that bug as fixed? If so, please go ahead.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: --- → verified
status-firefox-esr52: --- → unaffected
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.