1578991 - 9.17 - 16.31% raptor-tp6-twitch-firefox (windows10-64-shippable) regression on push b3ee5fed44ec711f79202150223cda9397c6ed80 (Wed August 14 2019)

Status: RESOLVED FIXED

(DevTools :: JSON Viewer, defect)

Description

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Raptor has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset= b3ee5fed44ec711f79202150223cda9397c6ed80

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

Regressions:

16% raptor-tp6-twitch-firefox windows10-64-shippable opt 132.95 -> 154.63
9% raptor-tp6-twitch-firefox windows10-64-shippable opt 129.99 -> 141.90

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

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

To learn more about the regressing test(s) or reproducing them, please see: https://wiki.mozilla.org/TestEngineering/Performance/Raptor

*** 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/TestEngineering/Performance/Talos/RegressionBugsHandling

Comment 1

[Alexandru Ionescu (needinfo me) [:alexandrui]]

This bisection revealed the regression.

Comment 2

[Sebastian Streich [:sstreich]]

Hey! 👋

Sorry for the regression.
The problem here is the Following:
Twitch.tv loads Documents with Nosniff enabled.
Prior to the Patch, we did not spin up any sniffer if nosniff was detected.
-> This broke the Json-Viewer - As this requires a special sniffer to be run in any case.

So the new patch now Lets each sniffer handle the nosniff flag by itself.
That also means we spin each sniffer up, even though it would always not have a result.

I will investigate this week if we can split the sniffers in "respects-nosniff" and !"respects-nosniff"
and call accordingly.

Comment 3

[Sebastian Streich [:sstreich]]

Attachment: Bug 1578991 - Skip Calling Sniffers in Case of NoSniff and JSON Mime r=ckerschb

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb8d46c7826b
Skip Calling Sniffers in Case of NoSniff and JSON Mime r=ckerschb,dragana

Comment 5

[Oriol Brufau [:Oriol]]

It seems the patch only handles application/json, application/manifest+json and text/json.

But actually the JSON Viewer loads for any application/*+json (bug 1388335), e.g. application/vnd.api+json, application/hal+json or application/ld+json. I guess these will break with NoSniff.

And the JSON Viewer doesn't load for text/json.

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/eb8d46c7826b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 8

[Oriol Brufau [:Oriol]]

Please fix bug 1583869 before uplifting this to beta.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Fx70 RC week is next week. If this is going to make it into 70, it needs an uplift request ASAP. Otherwise we should wontfix.

Comment 10

[Sebastian Streich [:sstreich]]

Hey! I just proposed 1585055 for uplift.
If this one gets uplifted - Nosniff will be disabled in 70 which is the root cause for this perf-regression, so we wont run into this unless a user manually activates it. So i guess this does not need an uplift and we can mark this as fixed :)

Comment 11

[Catalin Sasca, Desktop QA [:csasca]]

Hi Sebastian, is there something that we can manually verify here? Thanks.

Comment 12

[Sebastian Streich [:sstreich]]

Hey! The patch here just skips the sniffing in a few cases where sniffing would not make sense to do anyway.
I'm not sure if this is easy to manually verify.

Comment 13

[Catalin Sasca, Desktop QA [:csasca]]

Oh, ok then, thanks for the fast reply!