Skip to content

🚀 [Story performance] Experiment to load first page assets before loading other pages to improve LCP#34846

Merged
mszylkowski merged 29 commits into
ampproject:mainfrom
mszylkowski:px_firstpage
Jul 12, 2021
Merged

🚀 [Story performance] Experiment to load first page assets before loading other pages to improve LCP#34846
mszylkowski merged 29 commits into
ampproject:mainfrom
mszylkowski:px_firstpage

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski commented Jun 11, 2021 •
edited
Loading

In order to improve the Largest Contentful Paint which is the major failing component of the Core Web Vitals for stories, we can delay the loading of the assets (images, audios and videos) on the inactive pages until the active page (the first page that shows up when the story is loaded).

Since we use the distance attribute to determine what to load, following pages will not show up until the active page appears, or we navigate to another page.
lcp3

LCP lab data for this PR

image

LCP field data from WebPageTest

LCP with change 10.09s
LCP without change 11.07s

@mszylkowski mszylkowski requested a review from gmajoulet June 17, 2021 18:39
@mszylkowski mszylkowski marked this pull request as ready for review June 17, 2021 18:39
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot Bot commented Jun 17, 2021

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story-page.js
extensions/amp-story/1.0/amp-story.js
extensions/amp-story/1.0/test/test-amp-story.js

1 similar comment
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot Bot commented Jun 17, 2021

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story-page.js
extensions/amp-story/1.0/amp-story.js
extensions/amp-story/1.0/test/test-amp-story.js

gmajoulet
gmajoulet reviewed Jun 17, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@gmajoulet gmajoulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you launch this behind a CSI reported experiment so we can gather performance metrics?

Comment thread examples/amp-story/lcp.html
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
kristoferbaxter
kristoferbaxter reviewed Jun 17, 2021
View reviewed changes
Comment thread extensions/amp-story/1.0/amp-story-page.js Outdated
kristoferbaxter
kristoferbaxter reviewed Jun 17, 2021
View reviewed changes
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
@mszylkowski
Copy link
Copy Markdown
Contributor Author

mszylkowski commented Jun 17, 2021 •
edited
Loading

@kristoferbaxter will remove the logs 👍

kristoferbaxter
kristoferbaxter reviewed Jun 17, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments, but let's also get @jridgewell or @erwinmombay to take a look.

Perhaps for some period of time asking @ampproject/wg-performance for reviews on performance related changes to the stories format would be prudent.

@mszylkowski mszylkowski requested a review from gmajoulet June 18, 2021 15:33
gmajoulet
gmajoulet reviewed Jun 18, 2021
View reviewed changes
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
@gmajoulet
Copy link
Copy Markdown
Contributor

gmajoulet commented Jun 18, 2021

Can you clarify what you're measuring in the metrics you're posted? A LCP of 0.5s without any optimization is very very surprising to me

@mszylkowski mszylkowski self-assigned this Jun 22, 2021
@mszylkowski mszylkowski requested a review from gmajoulet June 22, 2021 16:43
gmajoulet
gmajoulet reviewed Jun 22, 2021
View reviewed changes
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
Comment thread extensions/amp-story/1.0/test/test-amp-story.js Outdated
Comment thread extensions/amp-story/1.0/test/test-amp-story.js Outdated
Comment thread extensions/amp-story/1.0/test/test-amp-story.js Outdated
Comment thread extensions/amp-story/1.0/test/test-amp-story.js Outdated
Comment thread extensions/amp-story/1.0/test/test-amp-story.js Outdated
@mszylkowski mszylkowski requested a review from gmajoulet June 23, 2021 14:52
@mszylkowski mszylkowski requested a review from gmajoulet June 24, 2021 20:33
gmajoulet
gmajoulet reviewed Jun 24, 2021
View reviewed changes
Comment thread extensions/amp-story/1.0/amp-story.js
@mszylkowski mszylkowski requested a review from gmajoulet June 24, 2021 23:11
jridgewell
jridgewell reviewed Jun 29, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems instead of "prealoding all" (parallel), it should just use serial preloading? Ie, prioritize based on distance.

Comment thread extensions/amp-story/1.0/amp-story.js
gmajoulet
gmajoulet reviewed Jun 29, 2021
View reviewed changes
Comment thread examples/amp-story/lcp.html Outdated
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
@mszylkowski mszylkowski requested a review from gmajoulet July 1, 2021 18:32
gmajoulet
gmajoulet approved these changes Jul 1, 2021
View reviewed changes
Comment thread examples/amp-story/lcp.html Outdated
@kristoferbaxter
Copy link
Copy Markdown
Contributor

kristoferbaxter commented Jul 2, 2021

Before moving forward here, as an additional verification can you provide a few runs (9 is likely ok) on WebPageTest of this diff on a midrange device?

@mszylkowski
Copy link
Copy Markdown
Contributor Author

mszylkowski commented Jul 2, 2021

@kristoferbaxter thanks for the suggestion, didn't know about WebPageTest. Added the results to the PR description but here they are:

LCP field data from WebPageTest

LCP with change 10.09s
LCP without change 11.07s

mszylkowski
mszylkowski commented Jul 2, 2021
View reviewed changes
Comment thread examples/amp-story/performance/lcp-compressed.html
jridgewell
jridgewell reviewed Jul 2, 2021
View reviewed changes
Comment thread extensions/amp-story/1.0/amp-story.js Outdated
Comment thread extensions/amp-story/1.0/amp-story-page.js
@mszylkowski mszylkowski requested a review from jridgewell July 2, 2021 20:45
@mszylkowski mszylkowski merged commit 1594e79 into ampproject:main Jul 12, 2021
@mszylkowski mszylkowski deleted the px_firstpage branch July 12, 2021 15:00
@mszylkowski mszylkowski changed the title 🚀 [Story performance] Load first page assets before loading other pages to improve LCP 🚀 [Story performance] Experiment to load first page assets before loading other pages to improve LCP Jul 12, 2021
@gmajoulet
Copy link
Copy Markdown
Contributor

gmajoulet commented Jul 12, 2021

@mszylkowski can you send a follow up PR to launch this to 10% plz? We can control further using the new instant experiment system later on once it's back, but for this one let's use the regular experiments.

@mszylkowski
Copy link
Copy Markdown
Contributor Author

mszylkowski commented Jul 12, 2021

For reference: Launching experiment to 10% in #35192

@mszylkowski mszylkowski mentioned this pull request Jul 19, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmajoulet gmajoulet gmajoulet approved these changes

@jridgewell jridgewell jridgewell approved these changes

@rcebulko rcebulko Awaiting requested review from rcebulko

@kristoferbaxter kristoferbaxter Awaiting requested review from kristoferbaxter

Assignees

@mszylkowski mszylkowski

Labels

PR use: In Beta / Experimental PR use: In Stable

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mszylkowski @gmajoulet @kristoferbaxter @jridgewell @rcebulko @ampprojectbot