1540765 - PersistentCache should do JSON parsing off of the main thread

Status: RESOLVED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

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

Here's a profile we gathered on the 2018 reference device:

https://perfht.ml/2YGBDLn

We're doing JSON parsing of something really big on the main thread.

Unfortunately, we don't have a great sense of what function is actually executing here, but from inspection, we suspect it's this:

https://searchfox.org/mozilla-central/rev/2738efcf98d746c8810819e7c0634b2c57212a8d/browser/components/newtab/lib/PersistentCache.jsm#61-64

This might be pretty simple to fix, if we use window.fetch to request the resource off of the disk off-main-thread, and then .json() to do the decoding off-main-thread.

Comment 1

[Ed Lee :Mardak]

If it is the persistent cache, it's probably accessing activity-stream.topstories.json from the profile cache directory which looks to be about 13MB for me…

Unclear if it should be that big, but at a glance, it has the domain affinities as well as bug 1497616 v2 personalization log priors, vocab_idfs, taggers, etc.

Comment 2

[Nan Jiang [:nanj]]

Yes, I guess this cache file is likely to be the one for Pocket personalization V2.

+jkoren and :thecount

Comment 3

[Scott [:thecount] Downe]

Some context.

So the initial load of the v2 data is done in either idle-daily, or when we load it from cache. I'm guessing the cache load is on the main thread and the idle-daily is not? But it's not immediately obvious to me how some of these events fire. This isn't something that we need right away, so doing it off the main thread, however that's possible, seems perfectly reasonable.

The main chunk of the data comes in as file attachments using remote settings. They are then used to create some objects that we can cache, and use these object to rehydrate the personalization. In the idle-daily case, we cache, then immediately move to creating the personalization object, and don't bother with rehydrating it. The personalization object we create or rehydrate is then used to score stories.

This also should be preffed off as it was an experiment up to this point.

Part of the difficulty with this test was the fact it was an experiment, and preffed off, we needed to be able to turn it off and on, interface more of less with the structure of v1, and be able to do this with as little changes to non preffed off code as we could. Given the results of the experiment (I'm not sure where that ended up, but I remember jkoren mention it was a wash and we needed to try again) we would then look into optimizations (without worrying as much with the above issues) once we had some confidence that it was better than v1 and worth moving forward with.

Comment 4

[Andrei Oprea [:andreio]]

PersistantCache is also used by TippyTop and TopStories so fixing this would be a win not just for Discovery Stream.
For my local profile topstories and tippytop are around 200KB which is almost 1/2 of the activity-stream bundle so it's significant.

Comment 5

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4879

Comment 6

[Nan Jiang [:nanj]]

Looks like there is another case of main-thread file read in v2 personalization.

There are dozens of files (20 MB) in that directory (OS.Path.join(OS.Constants.Path.localProfileDir, "personality-provider")). We might want to apply the same optimization there.

Comment 7

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/7a266c35cb30