Closed Bug 1770885 Opened 6 months ago Closed 6 months ago

8.39% twitter fcp (Linux) regression on Wed May 18 2022

Categories

(Core :: DOM: Core & HTML, defect)

defect

Tracking

()

RESOLVED FIXED
103 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox100 --- unaffected
firefox101 --- unaffected
firefox102 --- fixed
firefox103 --- fixed

People

(Reporter: afinder, Assigned: emilio)

References

(Regression)

Details

(Keywords: perf, perf-alert, regression)

Attachments

(5 files)

Perfherder has detected a browsertime performance regression from push 208368baa3eef0d10fea2952ba53f8703e4488d3. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
8% twitter fcp linux1804-64-shippable-qr fission warm webrender 186.02 -> 201.62

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the offending patch(es) will be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

For more information on performance sheriffing please see our FAQ.

Flags: needinfo?(emilio)

Set release status flags based on info from the regressing bug 1768198

Depends on: 1770910

The twitter page has:

<meta http-equiv="origin-trial" content="Ap6SMBNB0lQoXpXl4I9vyTJqJ7Y0X9tPd6Q6rN697iHdubQQxBcWHy21N3N7uEz7Ba5UKMbN+eLvDczBSbi27AsAAABfeyJvcmlnaW4iOiJodHRwczovL3R3aXR0ZXIuY29tOjQ0MyIsImZlYXR1cmUiOiJCYWRnaW5nIiwiZXhwaXJ5IjoxNTY0NTgyNzY2LCJpc1N1YmRvbWFpbiI6dHJ1ZX0=">
<meta http-equiv="origin-trial" content="Apir4chqTX+4eFxKD+ErQlKRB/VtZ/dvnLfd9Y9Nenl5r1xJcf81alryTHYQiuUlz9Q49MqGXqyaiSmqWzHUqQwAAABneyJvcmlnaW4iOiJodHRwczovL3R3aXR0ZXIuY29tOjQ0MyIsImZlYXR1cmUiOiJDb250YWN0c01hbmFnZXIiLCJleHBpcnkiOjE1NzUwMzUyODMsImlzU3ViZG9tYWluIjp0cnVlfQ==">
<meta http-equiv="origin-trial" content="AleGS26SZL7UA8Fe1DbvXzoay74bPTvrfKKGimIu1RI8vA+RtXOSVlizUkz2zU/fQoFoOTgCiCciP6pM5teaeQgAAABjeyJvcmlnaW4iOiJodHRwczovL3R3aXR0ZXIuY29tOjQ0MyIsImZlYXR1cmUiOiJTbXNSZWNlaXZlciIsImV4cGlyeSI6MTU3OTAyMDkyMSwiaXNTdWJkb21haW4iOnRydWV9">
Flags: needinfo?(emilio)
See Also: → 1770921

This takes half the time of the signature validation process, and we
trust the key we're providing, so we shouldn't need to do this.

Plus, PK11_VerifyWithMechanism verifies the key again
(see bug 1770921).

Assignee: nobody → emilio
Status: NEW → ASSIGNED

As verifying the signature can be slow-ish (filed bug 1770921) for this.

This makes the overhead of origin trial tokens negligible in the
attached test-case, as opposed to taking ~60ms.

Depends on D147171

As per recommendation in the other revision.

This avoids the key copy, and should prevent further verification when
using the returned key.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/319e6905f56c
Don't verify the origin-trials public key. r=smaug
https://hg.mozilla.org/integration/autoland/rev/54ab422e8b53
Do all the other sanity-checking before verifying the signature. r=hsivonen
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a21aa12623f
Cache existing public key rather than recreating it over and over. r=keeler
https://hg.mozilla.org/integration/autoland/rev/1ab65da024c4
Avoid key copy + re-verification in CreateECPublicKey. r=keeler

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

Comment on attachment 9277931 [details]
Bug 1770885 - Don't verify the origin-trials public key. r=smaug

Beta/Release Uplift Approval Request

  • User impact if declined: Perf regression on sites that use Chrome origin trials.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Just reorders and avoids some operations to be faster. Other fixes are probably not so critical to uplift.
  • String changes made/needed: none
  • Is Android affected?: Yes
Flags: needinfo?(emilio)
Attachment #9277931 - Flags: approval-mozilla-beta?
Attachment #9277932 - Flags: approval-mozilla-beta?

Comment on attachment 9277931 [details]
Bug 1770885 - Don't verify the origin-trials public key. r=smaug

Approved for 102 beta 5, thanks.

Attachment #9277931 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9277932 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Pulsebot from comment #10)

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a21aa12623f
Cache existing public key rather than recreating it over and over. r=keeler
https://hg.mozilla.org/integration/autoland/rev/1ab65da024c4
Avoid key copy + re-verification in CreateECPublicKey. r=keeler

== Change summary for alert #34332 (as of Tue, 07 Jun 2022 06:50:39 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
13% twitch fcp linux1804-64-shippable-qr fission warm webrender 58.90 -> 51.04
6% twitter fcp linux1804-64-shippable-qr fission warm webrender 198.08 -> 186.00
5% twitter fcp windows10-64-shippable-qr fission warm webrender 101.85 -> 96.79
4% twitter fcp windows10-64-shippable-qr cold fission webrender 196.21 -> 188.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34332

You need to log in before you can comment on or make changes to this bug.