Closed Bug 1525494 Opened 7 months ago Closed 6 months ago

load all DiscoveryStream component feeds in parallel rather than serially

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 67
Iteration:
67.1 - Jan 28 - Feb 10
Tracking Status
firefox66 + verified
firefox67 --- verified

People

(Reporter: dmose, Assigned: thecount)

References

Details

(Keywords: github-merged, perf)

Attachments

(2 files, 1 obsolete file)

Instead of loading all the feeds one after another, we should load them in parallel so that they finish sooner and things display sooner.

[Tracking Requested - why for this release]: Performance fixes so we don't regress what it's like on release for the test.

Assignee: dmose → sdowne
Attachment #9042005 - Attachment is obsolete: true
Blocks: 1527504
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Can you add the uplift request / test steps for this and ask QA to verify?

Flags: needinfo?(sdowne)

Comment on attachment 9043736 [details]
Bug 1525494 - Parallel component feed loads

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

None

User impact if declined

Experiment results and user experience because of performance.

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

Needs manual test from QE?

Yes

If yes, steps to reproduce

  1. Set the pref browser.newtabpage.activity-stream.discoverystream.config to {"enabled":true,"show_spocs":false,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=40249-e88c401e1b1f2242d9e441c4&layout_variant=dev-test-feeds"}
  2. Load a new tab.

It should load a few seconds faster than before the patch. In my tests I got around 18 seconds without the patch and 10 with, which is pretty noticeable. That link is designed to be slow as it loads a bunch of feeds. There is caching so the second attempt should be significantly faster.

List of other uplifts needed

None

Risk to taking this patch

Medium

Why is the change risky/not risky? (and alternatives if risky)

Behind pref

String changes made/needed

None

Flags: needinfo?(sdowne)
Attachment #9043736 - Flags: approval-mozilla-beta?
Flags: needinfo?(bnagabandi)

QA Results:

Tested on :

FF Nightly version : 67.0a1 (2019-02-15) - This is the latest build while I was testing.
OS : Mac and Windows 10 Pro

Mac OS:
I did not notice much delay for the feeds to load. Response time was ~3seconds, and 2nd attempt was pretty fast.

Windows OS:
I could see the new tab loaded after 12seconds. 2nd attempt was immediate.

Flags: needinfo?(bnagabandi)

I think this is expected results. 12 seconds seems roughly close to the expected time of 10 seconds.

Thanks Scott.

Additional note. I have this Windows 10 Pro on my VM.

Works as expected.

Marking as verified.

Status: RESOLVED → VERIFIED

Comment on attachment 9043736 [details]
Bug 1525494 - Parallel component feed loads

Planned work for pocket/new tab. Verified in Nightly.
OK for beta uplift, should land for beta 9.
Landing order: bug 1519879, bug 1525494, bug 1526861, bug 1524669, bug 1527195, bug 1525391, bug 1527347, bug 1525366, bug 1527626, bug 1527397, bug 1518258, bug 1527701, bug 1527370.

Attachment #9043736 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Brahmini Nagabandi from comment #11)

Thanks Scott.

Additional note. I have this Windows 10 Pro on my VM.

Works as expected.

Marking as verified.

Can you please verify this issue on Firefox 66 Beta 9 (https://archive.mozilla.org/pub/firefox/candidates/66.0b9-candidates/build1/)?

Flags: qe-verify+
Flags: needinfo?(bnagabandi)

BETA Testing:

QA Results:

Tested on :

FF Beta version : 66.0b9 (64-bit)
OS : Mac and Windows 10 Pro
Date : Feb 21

**Mac OS:
I did notice the response time was ~6seconds, and 2nd attempt was pretty fast.

Windows OS:
I could see the new tab loaded at ~20seconds. 2nd attempt was fast and took ~5seconds.

Scott - can you confirm if this is expected?

Flags: needinfo?(bnagabandi) → needinfo?(sdowne)

Yeah that sounds reasonable for that layout/feed.

Flags: needinfo?(sdowne)

Marking the status-firefox66 tracking flag as verified based on previous comments.

Thanks for verifying this as well!

Flags: qe-verify+

(In reply to Camelia Badau [:cbadau], Release Desktop QA from comment #17)

Marking the status-firefox66 tracking flag as verified based on previous comments.

Thanks for verifying this as well!

Thank you.

Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.