1434213 - 16.19 - 19.2% Stylo Servo_DeclarationBlock_SetPropertyById_Bench / Stylo Servo_DeclarationBlock_SetPropertyById_WithInitialSpace_Bench (windows7-32) regression on push 4f292fa4e4a3 (Thu Jan 25 2018)

Status: RESOLVED WONTFIX

(Core :: DOM: Workers, defect, P2)

Description

[Ionuț Goldan [:igoldan]]

We have detected a platform microbenchmarks regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e74671f1779848ba07bf488d52da0782b8a579ae&tochange=4f292fa4e4a30c8ba0745cf9884dc08c098752c0

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

 19%  Stylo Servo_DeclarationBlock_SetPropertyById_WithInitialSpace_Bench windows7-32 opt      526,690.50 -> 627,816.00
 16%  Stylo Servo_DeclarationBlock_SetPropertyById_Bench windows7-32 opt                       523,743.42 -> 608,524.33


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=11302

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 jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Platform_Microbenchmarks

Comment 1

[Ionuț Goldan [:igoldan]]

:baku It looks like bug 1425458 regressed these 2 tests on Windows 7 platform. Can you please investigate this issue, for a fix? I'm sorry it's a little old.

Comment 2

[Andrea Marchesini [:baku]]

These patches do not touch window objects at all. It seems unrelated. How is the regression range calculated?
smaug, do you have any idea how this regression is connected with PerformanceStorageWrapper?

Comment 3

[Ionuț Goldan [:igoldan]]

(In reply to Andrea Marchesini [:baku] from comment #2)
> These patches do not touch window objects at all. It seems unrelated. How is
> the regression range calculated?

Comment 4

[Ionuț Goldan [:igoldan]]

We described the Stylo tests here: https://wiki.mozilla.org/Buildbot/Talos/Tests#Stylo_gtest_microbenchmarks

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

really odd regression. Is it possible to push backout to tryserver to run servo tests there to see if it helps?

Comment 6

[Andrea Marchesini [:baku]]

igoldan can you please try to see if backing it out on a try push fixes the regression?

Comment 7

[Ionuț Goldan [:igoldan]]

(In reply to Andrea Marchesini [:baku] from comment #6)
> igoldan can you please try to see if backing it out on a try push fixes the
> regression?
Surely. I'll keep you informed.

Comment 8

[Ionuț Goldan [:igoldan]]

I think bug 1425458 has been written over by other changesets.
So I pushed to Try the previous changeset, to see if the regression reproduces there also.

Comment 9

[Ionuț Goldan [:igoldan]]

I managed to reproduce this regression on Try [1].

[1] https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=8aa35fa30405&newProject=try&newRevision=d66c298ff696&framework=6&filter=stylo%20windows7-32

Comment 10

[Andrea Marchesini [:baku]]

Without bug 1425458?

Comment 11

[Ionuț Goldan [:igoldan]]

(In reply to Andrea Marchesini [:baku] from comment #10)
> Without bug 1425458?

With bug 1425458, which is on the right side of the comparison view.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

It makes no sense for that workers patch to actually affect that microbenchmark meaningfully, I'd think...

Comment 13

[Bobby Holley (:bholley)]

Yeah, this is quite weird, but try does seem to be giving reproducible results. I think we should at least try to understand what's going on here.

igoldan, can you push a few more revisions to bisect the patches? It would be very interesting to know whether the slowdown comes with part 0, part 1, part 2, etc.

Comment 14

[Ionuț Goldan [:igoldan]]

(In reply to Bobby Holley (:bholley) from comment #13)
> Yeah, this is quite weird, but try does seem to be giving reproducible
> results. I think we should at least try to understand what's going on here.
> 
> igoldan, can you push a few more revisions to bisect the patches? It would
> be very interesting to know whether the slowdown comes with part 0, part 1,
> part 2, etc.

Ok, will do.

Comment 15

[Ionuț Goldan [:igoldan]]

It turns out the slowdown comes from part 0 and then persists on the following parts.
This is a comparison between _non-bug 1425458_ and _part 0 of bug 1425458_ [1].

[1] https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=8aa35fa30405&newProject=try&newRevision=0bb5ee3aec78&framework=6&filter=stylo

Comment 16

[Andrea Marchesini [:baku]]

part 0 of bug 1425458 introduces a null param in NS_NewChannel. I don't see any relationship between that patch and this regression, but let's see what bholley says.

Comment 17

[Bobby Holley (:bholley)]

It looks like we've seen several other cases where gtest microbenchmarks are sensitive to entirely-unrelated changes on windows only. See bug 1415465 and bug 1429319 for examples.

igoldan, can you verify that these tests are running on mac and linux, and that we don't see a corresponding regression on those platforms? If so, I'm inclined to WONTFIX this.

Comment 18

[Ionuț Goldan [:igoldan]]

(In reply to Bobby Holley (:bholley) from comment #17)
> It looks like we've seen several other cases where gtest microbenchmarks are
> sensitive to entirely-unrelated changes on windows only. See bug 1415465 and
> bug 1429319 for examples.
> 
> igoldan, can you verify that these tests are running on mac and linux, and
> that we don't see a corresponding regression on those platforms? If so, I'm
> inclined to WONTFIX this.

These tests do run on all desktop platforms. Indeed, there aren't any regressions
on the other platforms. Even more, OS X notes some improvements.

Comment 19

[Bobby Holley (:bholley)]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #18)
> (In reply to Bobby Holley (:bholley) from comment #17)
> > It looks like we've seen several other cases where gtest microbenchmarks are
> > sensitive to entirely-unrelated changes on windows only. See bug 1415465 and
> > bug 1429319 for examples.
> > 
> > igoldan, can you verify that these tests are running on mac and linux, and
> > that we don't see a corresponding regression on those platforms? If so, I'm
> > inclined to WONTFIX this.
> 
> These tests do run on all desktop platforms. Indeed, there aren't any
> regressions
> on the other platforms. Even more, OS X notes some improvements.

Are these improvements similarly arbitrary to the regressions we see on windows? My hope was that the factors causing the numbers to jump around were limited to MSVC, and thus we could use the other platforms to measure regressions more reliably.

In any case, sounds like this is WONTFIX.

Comment 20

[Ionuț Goldan [:igoldan]]

(In reply to Bobby Holley (:bholley) from comment #19)
> Are these improvements similarly arbitrary to the regressions we see on
> windows? My hope was that the factors causing the numbers to jump around
> were limited to MSVC, and thus we could use the other platforms to measure
> regressions more reliably.

No, these improvements aren't arbitrary, don't worry about that. We can trust other platforms.

If you want to know more, this is one OS X improvement [1]; it's was influenced by
an update on the rust builders.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1418081

Comment 21

[Ionuț Goldan [:igoldan]]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #20)
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1418081

And this is the performance alert associated with bug 1418081: https://treeherder.mozilla.org/perf.html#/alerts?id=11278

Comment 22

[Bobby Holley (:bholley)]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #20)
> (In reply to Bobby Holley (:bholley) from comment #19)
> > Are these improvements similarly arbitrary to the regressions we see on
> > windows? My hope was that the factors causing the numbers to jump around
> > were limited to MSVC, and thus we could use the other platforms to measure
> > regressions more reliably.
> 
> No, these improvements aren't arbitrary, don't worry about that. We can
> trust other platforms.

Ok, that's great news! In that case I don't personally object to turning off the microbenchmarks on windows if we think they're too noisy to be worthwhile.

Comment 23

[Ionuț Goldan [:igoldan]]

(In reply to Bobby Holley (:bholley) from comment #22)
> (In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment
> #20)
> > (In reply to Bobby Holley (:bholley) from comment #19)
> > > Are these improvements similarly arbitrary to the regressions we see on
> > > windows? My hope was that the factors causing the numbers to jump around
> > > were limited to MSVC, and thus we could use the other platforms to measure
> > > regressions more reliably.
> > 
> > No, these improvements aren't arbitrary, don't worry about that. We can
> > trust other platforms.
> 
> Ok, that's great news! In that case I don't personally object to turning off
> the microbenchmarks on windows if we think they're too noisy to be
> worthwhile.

This sounds like a new bug. Do you agree with us turning off microbenchmarks on Windows?

Comment 24

[Joel Maher ( :jmaher ) (UTC -8)]

I am in favor of turning off some (all if needed) gtests on windows.