Closed Bug 1508944 Opened 6 years ago Closed 6 years ago

2.23 - 3.47% tp5o / tp5o_webext (linux64, linux64-qr, osx-10-10, windows10-64, windows10-64-qr, windows7-32) regression on push 209ef31926beef24da82d91223a89d26c712eccc (Tue Nov 20 2018)


(Firefox :: Protections UI, defect, P2)




Firefox 65
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed


(Reporter: igoldan, Assigned: ehsan.akhgari)



(Keywords: perf, regression, talos-regression)


(1 file)

Talos has detected a Firefox performance regression from push:

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


  3%  tp5o windows10-64-qr opt e10s stylo         129.63 -> 134.14
  3%  tp5o windows10-64 opt e10s stylo            133.47 -> 137.43
  3%  tp5o osx-10-10 opt e10s stylo               229.17 -> 235.77
  3%  tp5o_webext windows10-64 opt e10s stylo     196.32 -> 201.94
  3%  tp5o windows10-64 pgo e10s stylo            119.80 -> 123.10
  2%  tp5o_webext windows7-32 pgo e10s stylo      174.54 -> 178.72
  2%  tp5o_webext windows7-32 opt e10s stylo      193.78 -> 198.21
  2%  tp5o linux64 pgo e10s stylo                 127.14 -> 130.01
  2%  tp5o linux64 opt e10s stylo                 141.44 -> 144.60
  2%  tp5o_webext linux64-qr opt e10s stylo       213.89 -> 218.65

You can find links to graphs and comparison views for each of the above tests at:

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:

For information on reproducing and debugging the regression, either on try or locally, see:

*** 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:
Component: General → Tracking Protection
Product: Testing → Firefox
Flags: needinfo?(ehsan)
I'll soon post the Gecko profiles.
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #2)

> For tp5o on OS X:
> after:
> net%2Fv1%2Ftask%2FQHDqWRFKT8eTRG8C4zhjPg%2Fruns%2F0%2Fartifacts%2Fpublic%2Fte
> before:
> net%2Fv1%2Ftask%2FW84l9lDVRBuUPRJzGF0vVw%2Fruns%2F0%2Fartifacts%2Fpublic%2Fte

Oh, sorry. Got this wrong. For OS X, the correct profiles are these:


Thanks for the report.  This is expected in the sense that we are now doing more work, but it's also a huge regression which I'm not in a rush to just accept.  I'm discussing how to deal with this with my colleagues.  Maintaining the needinfo.
The patch attached is my attempt at fixing this regression without backing out the offending patch itself.  The approach is simple: if the top-level page already has the STATE_COOKIES_LOADED flag, just record the flag on our document and don't notify anything.  I came up with this approach when I noticed the content processes in the profiles after the change experiencing additional time running the OnSecurityChange callback coming from the nsGlobalWindowOuter::NotifyContentBlockingState() call site.

I pushed a couple of try pushes to verify that the regression does indeed go away:


Talos compare:
OK, the first approach was promising:


But this wasn't completely recovering the performance we lost due to my original landing.  Then I decided to even go one step further and apply the same idea to everything that this function does and that indeed recovers all of the lost perf, and indeed buys a little bit more here or there too:


The updated patch is ready for review now.
Flags: needinfo?(ehsan)
Assignee: nobody → ehsan
Priority: -- → P2
Attachment #9026829 - Attachment description: Bug 1508944 - Avoid dispatching the OnSecurityChange notification repeatedly for STATE_COOKIES_LOADED notifications → Bug 1508944 - Avoid dispatching the OnSecurityChange notification repeatedly when nothing has changed
See Also: → 1509228
Pushed by
Avoid dispatching the OnSecurityChange notification repeatedly when nothing has changed r=baku
Increased the timeout from 500ms to 1s, seems to make the test pass...
Flags: needinfo?(ehsan)
Pushed by
Avoid dispatching the OnSecurityChange notification repeatedly when nothing has changed r=baku
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
perf alerts are in:
== Change summary for alert #17780 (as of Fri, 23 Nov 2018 23:58:06 GMT) ==


  6%  tsvg_static windows7-32 pgo e10s stylo      45.61 -> 43.02
  3%  tp5o windows7-32 opt e10s stylo             135.05 -> 130.70
  3%  tp5o windows10-64-qr opt e10s stylo         133.55 -> 129.44
  3%  tp5o_webext windows7-32 pgo e10s stylo      178.56 -> 173.46
  3%  tp5o windows7-32 pgo e10s stylo             121.51 -> 118.21
  3%  tp5o windows10-64 opt e10s stylo            137.12 -> 133.52
  2%  tp5o_webext windows10-64 pgo e10s stylo     180.69 -> 176.47
  2%  tp5o_webext windows10-64 opt e10s stylo     201.43 -> 196.90

For up to date results, see:

But why did tsvg_static improve?
oh, that is marked as "invalid", it is a noisy test affected by lower/higher volumes of pushes.
Depends on: 1511477
You need to log in before you can comment on or make changes to this bug.