Skip to content

MWPW-206311: preload first-section LCP content before MEP - #6643

Open
vhargrave wants to merge 15 commits into
stagefrom
vhargrave/lcp-blocks-only-pre-mep
Open

MWPW-206311: preload first-section LCP content before MEP#6643
vhargrave wants to merge 15 commits into
stagefrom
vhargrave/lcp-blocks-only-pre-mep

Conversation

@vhargrave

@vhargrave vhargrave commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

What

Preload the first section's block code + non-commerce autoblocks + placeholder sheets + icons during the MEP/Target await in loadArea, instead of after it, so the LCP element can start painting sooner. Commerce (merch/mas) autoblocks are excluded — their heavy dependency chain slows pages whose hero isn't a store block.

Wins (measured on live www.adobe.com, Slow-4G + 4× CPU)

  • Mobile: net ~48s faster LCP across 8 locale/geo setups × 28 pages — wins beat losses ~7.5×, and every locale improved.
  • Desktop: net ~24s.
  • Biggest gains on pricing/plans and image-hero pages (firefly/plans ~−1.7s, creativecloud/plans, acrobat pricing +0.4–0.8s).
  • Only regressions: a few localized text-hero pages ~60–100ms (and /creativecloud.html ~80ms — follow-up).

Full analysis + raw crawl data (zips): MWPW-206183.

Kill switch

Emergency opt-out (default is enabled): <meta name="disable-mep-perf-optimization" content="on"> skips the preload entirely. Works per-page or site-wide via the global metadata sheet.

Test

To test performance you'll want to go to adobe.com pages and override the utils.js with the one in this PR.

You'll also want to test that pages still look the same (they should in theory and unit tests are all passing, but just to be safe)

Resolves: MWPW-206311

Victor Hargrave and others added 2 commits September 2, 2026 13:31
Warm the first (LCP) section's assets — authored blocks, link-autoblocks,
placeholders, and icons.js — in loadArea immediately before the MEP await
in checkForPageMods() holds up section processing, so their fetches are
already in flight when processing resumes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Warm the first section's block code, non-merch/mas autoblocks, placeholder sheets and icons during checkForPageMods() instead of after it, so the LCP element can paint sooner. fetchpriority=high; commerce (merch/mas) autoblocks excluded (heavy dep chain hurts non-store heroes).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aem-code-sync

aem-code-sync Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

Emergency opt-out: <meta name="disable-mep-perf-optimization" content="on"> skips the pre-MEP preload (page-level or site-wide via the global metadata sheet). Default = enabled.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…PageMods

high == browser default for these preload types, so the explicit fetchpriority plumbing is removed (preloadBlockResources reverts to its original signature). The preload now fires inside checkForPageMods right after the mep=off early return, co-locating it with MEP logic while still running before the MEP await.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…clusion)

Export preloadLcpCodeFiles and add focused tests: kill-switch metadata,
authored-block preload, non-commerce autoblock warming with merch/mas
excluded, media_*.mp4 video guard (anchor + img alt), placeholder and
icon gating, and the no-first-section early return.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move getValidatedMasLibsUrl, getMasDepUrl, STATIC_BLOCK_DEPS, blockDeps,
preloadBlockResources and preloadLcpCodeFiles above checkForPageMods so the
pre-MEP preloadLcpCodeFiles() call no longer needs an eslint-disable for
no-use-before-define. Pure reorder, no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-only-pre-mep

# Conflicts:
#	libs/utils/utils.js
…e-MEP)

#6589 hoisted geo-ip sheet warming into loadArea, but it runs after
`await checkForPageMods()` — i.e. behind the MEP/Target wait, the exact
latency this LCP work removes. Fold the same warmGeoIpSheet(config) call
into preloadLcpCodeFiles alongside the placeholder warms so a
{{...-geo-ip}} LCP token (or geo-ip-lcp MEP flag) is warmed before the
await. Idempotent via geoIpWarm[url] ??=, so loadArea's later call
dedupes. Geo-ip helpers hoisted above preloadLcpCodeFiles to keep it in
scope. Adds two tests (warms when lingo active + token; no-op otherwise).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both callers (preloadLcpCodeFiles and loadArea) repeated the same
lingoActive + -geo-ip-token + geo-ip-lcp gate. Move it inside
warmGeoIpSheet(config, section, isDoc) so each caller is a single line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
loadLink's dedup selector is rel-aware (link[href][rel=stylesheet] for
loadStyle vs link[href] for preload), so the preload does not shadow the
rel=stylesheet that decorateIcons later appends -- both coexist and the
browser consumes the preload (single fetch). Warm the render-affecting
sheet in parallel alongside icons.js.

Tighten two decorateIcons tests to query the stylesheet specifically now
that a preload link with the same href also exists.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The /merch|^mas-/ filter was applied only to link-derived autoblocks, so
an authored <div class="merch"> in the first section still pulled the
heavy commerce dep chain (commerce.js + autoblock.js) before MEP -- the
opposite of the change's intent. Apply the same exclusion to authored
blocks. Adds a test covering the authored-block path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vhargrave
vhargrave requested review from a team, markpadbe, sukamat and vgoodric September 4, 2026 10:01
preloadLcpCodeFiles ran preloadBlockResources before MEP, which applied the
DEFAULT block stylesheet via loadStyle. For a block MEP redirects with
useBlockCode (e.g. marquee -> libs/mep/sample-block-code/marquee, whose css
is `body { color: purple }`), the default blocks/marquee/marquee.css
(`.marquee { color: var(--color-white) }`, higher specificity) then overrode
the custom styling -> text rendered white, failing the cross-browser
mep-actions useBlockCode test.

Add a warmStyles option to preloadBlockResources: when set, css is warmed as
`rel=preload as=style` instead of applied via loadStyle. preloadLcpCodeFiles
now passes it, so the pre-MEP path only warms resources (nothing applied) and
decoration applies the correct (MEP-redirected) sheet post-MEP. Unit test
asserts the css is preloaded, not stylesheet-applied.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mokimo

mokimo commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review — PR #6643 (MWPW-206311: preload first-section LCP content before MEP)

Size: +240/-89 across 2 files (libs/utils/utils.js, test/utils/utils.test.js). Base: stage. Not oversize, well within convention. Most of the diff churn is a relocation (moving getValidatedMasLibsUrl/getMasDepUrl/STATIC_BLOCK_DEPS/preloadBlockResources/geoIpSiteKey/warmGeoIpSheet earlier in the file so the new preloadLcpCodeFiles can call them from checkForPageMods), not net-new logic.

No blockers. This is a clean, well-scoped, well-tested change. Details below for the record.

What I checked

  • Structure/conventions: no new block/feature directory, so CODEOWNERS is N/A. No new runtime deps, no console.*, no leftover debug code.
  • The loadLink preload→stylesheet interaction (the thing I'd worry about most with a "preload now, apply later" pattern): preloadBlockResources(..., { warmStyles: true }) only ever issues rel=preload as=style for CSS in the new pre-MEP pass, never loadStyle. Actual application still happens later, once, via processSection's existing preloadBlockResources(section.blocks) call. loadLink's dedup selector is stylesheet-aware (rel==='stylesheet' → link[href][rel="stylesheet"], else link[href]), so a stylesheet load after a preload for the same href correctly creates a second, real rel=stylesheet link rather than being swallowed as a no-op — confirmed by reading loadLink on stage directly, and by the "loadLink stylesheet dedup" + updated icons.css tests (test/utils/utils.test.js:1618,1644) that pin exactly this. (Commit history shows this was iterated on — "warm (don't apply) block css pre-MEP; fix useBlockCode nala" — so it looks like this exact risk was already found and fixed during development.)
  • Ordering: preloadLcpCodeFiles() is a synchronous, side-effect-only call inserted in checkForPageMods right after the mepParam === 'off' guard and before the MEP-enablement checks/awaits — so it fires before any of the slower MEP/Target/martech async work, matching the PR description.
  • Kill switch: disable-mep-perf-optimization metadata is checked first thing in preloadLcpCodeFiles, per-page or sitewide via global metadata. Good rollback lever.
  • Commerce exclusion: both authored blocks (div[class]) and link-derived autoblocks are filtered through isCommerceBlock, and getMasDepUrl's pre-existing hostname validation is untouched — no new security surface.
  • Test coverage: 13 new tests for preloadLcpCodeFiles (kill switch, marquee JS+warmed-but-unapplied CSS, autoblock detection incl. merch/mas exclusion, video-from-alt, placeholders tokens, icons, geo-ip warm gated on lingo) plus 2 existing icons tests tightened to assert the real rel="stylesheet" link specifically. Good coverage for a perf-sensitive change.

Nice-to-haves (non-blocking)

  1. libs/utils/utils.js:2314isCommerceBlock = (name) => /merch|^mas-/.test(name). The ^ only anchors the mas- alternative, so merch matches anywhere in the block name, not just as a prefix. Today's actual block names (merch, merch-card*, mas-*) make this harmless, but /^merch|^mas-/ would say what's actually meant and remove a future footgun if an unrelated block name ever contains "merch" as a substring.
  2. preloadLcpCodeFiles's auto-block detector (autoBlocks.find((c) => isTrustedAutoBlock(...))) is a simplified stand-in for the real decorateAutoBlock matcher, which has extra gating (pdf-viewer text check, fragment extension/self-href/mep-lingo-swap logic). That's fine for a speculative preload — worst case is one wasted preload request, never a functional break — but a one-line comment noting it's intentionally approximate would save a future reader from trying to keep the two byte-for-byte in sync as decorateAutoBlock evolves.
  3. Worth confirming intent: checkForPageMods returns before calling preloadLcpCodeFiles() when ?mep=off (i.e. the early if (mepParam === 'off') return; in libs/utils/utils.js:2347-2348 precedes the new call). Since this optimization isn't really MEP-specific (it preloads ordinary block/icon/placeholder resources), anyone perf-testing with ?mep=off won't see any of the measured wins from this PR. If that's deliberate, fine — just flagging in case it's accidental coupling.

Test plan

PR description includes the standard before/after aem.page/aem.live pairs across several properties (adobe.com, upp, da-cc, da-dc, da-express-milo), plus measured Slow-4G/4×CPU LCP deltas across 8 locale/geo × 28 pages with raw crawl data linked in MWPW-206183. Unit suite covers the new function thoroughly. No NALA-specific test file needed here (no new block).


No blockers found — approving in spirit (posting as a plain comment per team convention, not a formal GitHub review approval).

@mokimo mokimo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We ran over this in person conceptually, and I think the idea is great. Will be great to observe this in the real world and observe the LCP data accordingly

@overmyheadandbody overmyheadandbody left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly looking good, I just have the placeholders note for now that might be worth looking into

Comment thread libs/utils/utils.js Outdated

if (/{{|%7B%7B/.test(firstSection.innerHTML) && config.locale?.contentRoot) {
loadLink(`${base}/features/placeholders.js`, { rel: 'modulepreload', crossorigin: 'anonymous' });
getPlaceholderPaths(config).forEach((path) => loadLink(path, { rel: 'preload', as: 'fetch' }));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this might also need a crossorigin value. Comparing the before / after links for the homepage surfaces that there's a new additional request to placeholder.json, which is likely redundant, this might be the place to address that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed and fixed - added crossorigin: 'anonymous' to that loadLink call. as=fetch preloads only get reused by a later same-URL fetch when crossorigin matches (which is exactly why the other pre-existing as: 'fetch' preload in this file already sets it); without it the browser treats the preload as a mismatched resource and double-fetches, which is the extra request you spotted. See #6662.

Comment thread libs/utils/utils.js Outdated
Comment thread libs/utils/utils.js Outdated
Comment thread libs/utils/utils.js
* fix(perf): address review feedback on MWPW-206311 pre-MEP LCP preload

- add missing crossorigin to the placeholders.json preload link so the
  browser reuses it instead of double-fetching (overmyheadandbody)
- decouple preloadLcpCodeFiles() from ?mep=off - it preloads ordinary
  block/icon/placeholder resources, not MEP-specific ones, so only its
  own kill switch should gate it (mokimo, zagi25)
- anchor isCommerceBlock's merch alternative so a future block name
  containing "merch" as a substring isn't misclassified (mokimo)
- drop the unused catch binding to match the file's own convention
  (overmyheadandbody)
- note that the auto-block detector is intentionally approximate vs
  decorateAutoBlock (mokimo)

* fix(perf): drop added comments; keep mep=off short-circuit as-is

- no comments in libs/utils/utils.js, per repo convention
- revert the preloadLcpCodeFiles()/mepParam==='off' ordering: when MEP is off
  there's no MEP/Target await to hide the preload behind, and the normal
  block-loading path already loads everything fast enough on its own, so this
  only needs to run when MEP is present
@mokimo

mokimo commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Follow-up on the nice-to-haves from my review above, now that #6662 merged into this branch:

  1. isCommerceBlock anchor fixed (/merch|^mas-/ -> /^merch|^mas-/), with a regression test.
  2. Left the approximate-auto-block-detector note out - no new comments in libs/utils/utils.js per this repo's convention for that file.
  3. ?mep=off ordering: replied inline - kept as originally written, since when MEP is off there's no MEP/Target await to hide the preload behind and the normal path already loads fast enough without it.

Also fixed overmyheadandbody's crossorigin/redundant-request find and the unused catch binding - replied on those threads too. All green (215/215 Utils tests incl. new coverage, ESLint clean).

…6663)

fix(perf): drop JSDoc on getValidatedMasLibsUrl, no replacement comment

zagi25 flagged this JSDoc as inconsistent with the rest of the PR (#6643).
It was pre-existing code being relocated, not new authorship, but per repo
convention libs/utils/utils.js carries no comments - dropping it outright
rather than converting to a plain comment.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Reminder to set the Ready for Stage label - to queue this to get merged to stage & production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants