Closed Bug 1553375 Opened 5 years ago Closed 5 years ago

Remove react prerendering of placeholders (props, actions, telemetry)

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
Firefox 69
Iteration:
69.2 - May 27 - Jun 9
Tracking Status
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

(Keywords: github-merged)

Attachments

(1 file)

We most recently ran into a prerendering related bug 1553205 where the prerendered page results in components getting mounted with no content then later filling in when props update. That is indeed the expected behavior of prerendering placeholders, but it's an extra thing to keep in mind when implementing any component.

Prerendering was originally implemented as a performance optimization to improve time to first paint; however, our understanding of perceived performance is now to focus on time to settled content.

We recently got rid of a similar "wrong optimization focus" in bug 1500061 where the complexity of script loading optimization was no longer desired and slowed down development and introduced bugs.

Getting rid of prerendering would obviate bug 1485004 simplifying the transition to fluent while also removing a custom build step to get working for bug 1552007.

On the Discovery Stream side, there's bug 1544022 that would also be unnecessary and as described above, the prerendering step doesn't actually optimize the desired performance and potentially it slows things down with its complexity.

Type: task → enhancement
Iteration: --- → 69.1 - May 13 - 26
Priority: -- → P2
Iteration: 69.1 - May 13 - 26 → 69.2 - May 27 - Jun 9

With the profiler, I ran with discovery stream off then prerender pref true vs false. With false, I see (median of 3 runs) less running time in privileged process (289ms -> 261ms) and earlier topsites_first_painted_ts (2.254 -> 2.127s)

I just pushed some talos runs to check:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=5071a859e83134fe95ac7843f604dbfc919efb30&newProject=try&newRevision=f614d71d9042c95ed408726573ffff9cf78c9cfe

Nothing "important" showing up comparing prerender true vs false after 10 runs of each, but here's some low confidence linux64-qr results:

startup_about_home_paint 0.23% improvement
tabpaint 1.58% improvement
tart 0.11% improvement

No longer blocks: 1552007
Priority: P2 → P1

Making this more specific in cleaning up props (content-src/), actions (common/) and telemetry (lib/)

For props, no need for isPrerendered in Base.jsx, etc.

There is a lib/ActivityStream definition for the pref that maybe should just be removed as part of bug 1555448.

Assignee: nobody → edilee
Summary: Remove react prerendering of placeholders → Remove react prerendering of placeholders (props, actions, telemetry)
Blocks: 1555507
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: github-merged
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: