Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Creating empty maps while deleting non-existent cookies from SessionCookies.jsm's CookieStore causes jank near startup

RESOLVED FIXED in Firefox 55



Session Restore
4 months ago
2 months ago


(Reporter: florian, Assigned: ttaubert)


(Blocks: 2 bugs)

Firefox 55
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed)


(Whiteboard: [photon-performance])


(1 attachment)



4 months ago
See this profile where 149ms of jank are spent deleting cookies from _updateCookie in SessionCookies.jsm:

This is the code at added in bug 1239671 for Firefox 45 and later.

This happens the first time the current session is saved (so a few seconds after the initial startup) while calling _ensureInitialized in SessionCookies.jsm.

The cookies being deleted here are the non-session cookies, which obviously aren't in the session cookies store, so for each of them _ensureMap uselessly creates empty map objects.

I think we should create a different code path for initializing SessionCookie.jsm, so that we can avoid calling _updateCookie here.

note: This jank can't be reproduced with a fresh profile, as its length depends on how many non-session cookies are stored in the profile.

Comment 1

4 months ago
Ahhhh, yeah. This is terrible. I'll take it.
Assignee: nobody → ttaubert
Blocks: 1330635

Comment 2

4 months ago
Created attachment 8854794 [details] [diff] [review]
Attachment #8854794 - Flags: review?(mdeboer)
Comment on attachment 8854794 [details] [diff] [review]

Review of attachment 8854794 [details] [diff] [review]:

LGTM, but you'll need to rebase your things in bug 912717 if you land this before that one. Up to you.
Attachment #8854794 - Flags: review?(mdeboer) → review+

Comment 4

4 months ago
Pushed by
Don't create maps for non-session cookies when reloading all cookies r=mikedeboer

Comment 5

4 months ago
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55


3 months ago
Iteration: --- → 55.3 - Apr 17
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon] → [photon-performance]


3 months ago
Flags: qe-verify? → qe-verify-
We're trying to find the root cause of a 1.7ms in the 95th percentile input event responsiveness telemetry probe from the parent process in bug 1362094 and this patch is one of the three possible culprits that we currently suspect as the possible root causes.

Tim, do you think this could be the possible cause of bug 1362094?  Would you be OK with a temporary backout of this bug to see if the regression is reclaimed on telemetry?
Blocks: 1362094

Comment 7

3 months ago
(In reply to :Ehsan Akhgari (super long backlog, slow to respond) from comment #6)
> Tim, do you think this could be the possible cause of bug 1362094?  Would
> you be OK with a temporary backout of this bug to see if the regression is
> reclaimed on telemetry?

At first glance it seems that it shouldn't be able to cause a regression like this. I'd definitely be OK with a backout but unfortunately bug 912717 rewrote most of the file and touched that part of the code too. So we'd have to backout both.
If you think this isn't likely to have caused the regression and especially if it requires backing out bug 912717, let's sit on top of this one for now and examine the other two bugs as potential culprits...

Thanks for checking. :-)


2 months ago
No longer blocks: 1348289


2 months ago
Blocks: 1348289
You need to log in before you can comment on or make changes to this bug.