1544578 - Avoid unnecessary TopStoriesFeed work when Discovery Stream is enabled

Status: RESOLVED FIXED

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

Description

[Ed Lee :Mardak]

I noticed that TopStoriesFeed does some work during startup even though the page is showing discovery stream:

• register with SectionsManager
• read/write with PersistentCache (might be addressed with bug 1544052)
• network requests for endpoints
• loading PersonalityProvider and other jsms
• transforming stories

Only the first item should be necessary right now (with the implementation of bug 1533601). The others are probably unnecessarily slowing down the rest of startup.

The complicated part is probably how to correctly initialize TopStoriesFeed if it actually needs to do stuff.

Comment 1

[Ed Lee :Mardak]

I'm trying a quick hack of returning early in TopStoriesFeed and avoiding importing various modules:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=45f149c8f9971e34d019e39c9a602ae94bbb16e3&selectedJob=240743708

Here's a profile without the hack from my development machine checking main process times searching for stacks:
https://profiler.firefox.com/public/3b6a35864e132811978c0eaf9360aa494a0b464a/calltree/?profileName=&published&search=activity-stream&v=3

activity-stream: 847ms
PersonalityProvider: 565ms
TopStoriesFeed: 58ms

And a profile with the hack:
https://profiler.firefox.com/public/93417068c5c035ddb1ee218f8c007c8957f49cea/calltree/?profileName=&published&search=activity-stream&v=3

activity-stream: 86ms
PersonalityProvider: 0ms
TopStoriesFeed: 2ms

So a big chunk is related to PersonalityProvider with 306ms getting the attachment, 193ms writing topstories with PersistentCache, and 152ms maybeDownloadAttachment which seems to calculate hash from file contents.

Comment 2

[Nan Jiang [:nanj]]

All the work in the PersonalityProvider is for Pocket personalization V2. Does (or will) DiscoveryStream need that for now? If not, I think we can disable it to avoid the overhead.

Loop in Jonathan.

Comment 3

[Jessilyn Davis]

Scott - is the LOE vs. ROI?

Comment 4

[Scott [:thecount] Downe]

Let's not do this this week, but it can be significant enough to be worth the work in certain situations.

I tested two profiles with DS turned on, 1 with ed's patch, 1 without (control). Non of the tests had DS cache, and I did two sets of tests on the control profile, one with AS cache, and one without.

The examples I have are potentially the best case and the worst case. This is because I tested with a bad machine using a good network, cache loads lose out to network requests.

For the case of fast internet and slow machine and using existing AS cache, we get a 31% increase. This is because DS is only using the network and AS is loading from disk. While this isn't really fair, it is a possible situation, which would likely be our biggest gain.

I also did a test where AS didn't have cache, and it used just the network, and I saw no gains. This is our worst case.

I think because our worst case isn't a regression and best case shows improvements, it's probably worth it in the future, but probably not our best bet for right now.

Comment 5

[Punam Dahiya [:pdahiya]]

Patch landed in master

https://github.com/mozilla/activity-stream/commit/ea405a2c940629178ed50f2c3d0caf806978e5a6

Comment 6

[Punam Dahiya [:pdahiya]]

Attachment: GitHub Pull Request

Comment 7

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/02f031c0f789