1374665 - Sending permissions for service workers during content process initialization can be very slow

Status: RESOLVED FIXED

(Core :: Permission Manager, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

I just noticed some jank in my browser and captured this profile:

http://bit.ly/2swvv8V

We're spending about 300ms sending permissions for service workers down to the content process here.

We should probably do less of that.

I sent the list of service workers I have registered to mystor for reference.

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Stop parsing principals during GetPermissionsForKey

This should change the logic to completely avoid parsing any principals during the hot codepath.

MozReview-Commit-ID: 28BCIqA2Kf2

Comment 3

[(no longer active)]

This needs to be backported to 55, I think.  The profile in comment 0 is from 27 service workers.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Stop parsing principals during GetPermissionsForKey

Did some minor fixups due to try failures :)

MozReview-Commit-ID: 28BCIqA2Kf2

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/44532a19e524
Stop parsing principals during GetPermissionsForKey, r=ehsan

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: BETA - Stop parsing principals during GetPermissionsForKey

The patch cherry-picked cleanly on beta, but I'm re-attaching it anyway :)

MozReview-Commit-ID: 28BCIqA2Kf2

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out because it or something else from the push caused bug 1375293, which was failing more frequently than we could handle.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cbeb4bc60daa798c60102102c15e51a5af94265a

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/44532a19e524

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/62899ab59159
Backed out changeset 44532a19e524 for test_group_mouseevents.html failures a=backout

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/13422d457a38
Stop parsing principals during GetPermissionsForKey, r=ehsan

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13422d457a38

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8880143 [details] [diff] [review]
BETA - Stop parsing principals during GetPermissionsForKey

Approval Request Comment
[Feature/Bug causing the regression]: bug 1353179
[User impact if declined]: Starting new content processes will be slower if you have a decent number of Service Workers.
[Is this code covered by automated tests?]: Not explicitly, but exercised heavily.
[Has the fix been verified in Nightly?]: Just landed
[Needs manual test from QE? If yes, steps to reproduce]: No 
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: Small risk comes from changing the way that we handle principals in the permission manager slightly. The permission manager is already quite conservative with its use of complex principals, so I don't think it is likely to have been super broken.
[String changes made/needed]: None

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8880143 [details] [diff] [review]
BETA - Stop parsing principals during GetPermissionsForKey

Should improve perf for service worker heavy pages - let's take this for beta 5.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/017218b8ecdc