1677399 - CRLite uses 25% of CPU once per minute

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[timidbit]

Attachment: Screenshot 2020-11-15 000358.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0

Steps to reproduce:

I was just using Firefox Nightly as usual. Last build, Windows 10 x64.
I noticed strange periodical CPU usage.

Actual results:

Once per minute Firefox was using 25% CPU for some time (see attached screenshot). I checked what it was doing. There was some I/O using, \AppData\Roaming\Mozilla\Firefox\Profiles\<profile>\security_state\crlite.stash file in particular.
All is OK with new test profile.

Firefox Profiler results: https://share.firefox.dev/3f697LF

Expected results:

There should not be this unusual CPU usage.

Comment 1

[timidbit]

Attachment: Screenshot 2020-11-15 000522.png

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: Bug 1677399 - avoid re-downloading and re-processing CRLite filters/stashes that are already in cert_storage r?bbeurdouche

This patch uses nsICertStorage.hasPriorData() and a new local field on the
CRLite filter Remote Settings collection to avoid re-downloading and
re-processing CRLite filters and stashes that have already been put into
cert_storage.

Comment 3

[timidbit]

Attachment: image.png

It's permanent now.
This is the latest build at the moment, 20201119095716.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1fe88096bb3
avoid re-downloading and re-processing CRLite filters/stashes that are already in cert_storage r=bbeurdouche

Comment 5

[Atila Butkovits]

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

Comment 6

[Stefan Fleiter (:sfleiter)]

Will this be backported to Firefox 84?
Reducing CPU usage might be especially important especially with recent Chrome blog posts like https://blog.chromium.org/2020/11/tab-throttling-and-more-performance.html.

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Yes - this isn't enabled in release yet, but it will affect early betas.

Out of curiosity, timitbit, what is the value of services.settings.poll_interval in about:config?

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 9188457 [details]
Bug 1677399 - avoid re-downloading and re-processing CRLite filters/stashes that are already in cert_storage r?bbeurdouche

Beta/Release Uplift Approval Request

• User impact if declined: excess cpu usage/jank on early betas
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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): relatively small change, we have test coverage, and this isn't even enabled in release yet
• String changes made/needed: none

Comment 9

[timidbit]

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #7)

Yes - this isn't enabled in release yet, but it will affect early betas.

Out of curiosity, timitbit, what is the value of services.settings.poll_interval in about:config?

services.settings.poll_interval = 86400

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9188457 [details]
Bug 1677399 - avoid re-downloading and re-processing CRLite filters/stashes that are already in cert_storage r?bbeurdouche

Approved for 84.0b4.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0e5a3de7dd42