Closed Bug 1360277 Opened 3 years ago Closed 3 years ago
tp5 regression in November on windows 8
this is a regression that was overlooked in our automatic alerts due to a lot of change (regressions, fixes, etc.) in a really short time window. So we didn't get an alert. In looking back on tp5 over time, we see on November 23rd there is a regression that if fixed would give us a nice perf win. here is a zoomed in perfherder graph: https://treeherder.mozilla.org/perf.html#/graphs?timerange=31536000&series=%5Bmozilla-inbound,b68e2b084272409d7def3928a55baf0e00f3888a,1,1%5D&zoom=1477624922893.967,1481255587440.585,233.18839695142663,294.05796216881794 On this graph I had to hunt around and see that the increase happens here (since perfherder doesn't seem to show revision/alert history much past 4 months old): https://treeherder.mozilla.org/api/project/mozilla-inbound/resultset/147293/ typically I would retrigger this revision and some before/after to prove this, unfortunately I don't have the ability to retrigger since we don't keep artifacts >3 months. As we are focusing on windows 64 bit for Firefox 57, this is something we should put some attention on.
Tom can you please take a look at this changeset to see if it may have caused the regression?
Assignee: nobody → evilpies
Whiteboard: [qf] → [qf:p1]
Sure, I think it's possible, but maybe unlikely? What is perfherder actually running and can somebody test if we hit MappedArgumentsObject::obj_defineProperty. We should not hit this for assignments like `arguments[n] = 1`, but we might in some other cases.
This is the test: https://wiki.mozilla.org/Buildbot/Talos/Tests#tp5
I ran `./mach talos-test --suite tp5o --tppagecycles 2` and for every page-load I observed about 5 calls to that function, that should be insignificant. Maybe we disabled some other IC other optimization by adding this hook?
https://treeherder.mozilla.org/#/jobs?repo=try&revision=835b8fb0f5a5be40f0d7fbfbada746b969a96b28 patch backed out: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bba94caf757644b0bca0895d6edd1ad53992c9f8 I don't see a difference on tp5o.
a lot has changed since November 8th in the product- maybe there are no quick wins on this regression.
Joel, even though we don't keep artifacts around, can we create try builds with the before/after changesets at the time and re-run the tests to validate the jump?
I have attempted some pushes to try, if the builds work I will post the links here. A lot has changed since November, so I am lacking confidence in this, but enough confidence that I think a try push is worth a go :)
the builds worked, now to see if talos runs ok: https://firstname.lastname@example.org&fromchange=afd1637b8e0520d31295a16e17ba91a065767ed0&tochange=b1bc2c004763c7e1f429e56eaabc14d7462402fc we should have results in 8-12 hours when the jobs get scheduled.
oh, this is a light day for load on the trees! Here is what I see: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=b1bc2c004763&newProject=try&newRevision=afd1637b8e0520d31295a16e17ba91a065767ed0&framework=1&showOnlyImportant=0 That is a 4% tp5o e10s regression on win8; so given that backing that out on the tip didn't produce an improvement, then I see little value in pursuing this avenue- it is good to know that this did cause a specific regression in the past, possible there are lessons there.
I just realized I didn't backout the whole change, there was some change to freezing as well. I am going to do another push with a complete backout.
Any more data from the complete backout pushes Tom?
Apart from one outlier I don't see any improvements.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.