Skip to content

perf(player): avoid DomCrawler parse on every singular pageview#585

Merged
galbus merged 1 commit into
mainfrom
claude/amazing-hertz-2a6671
Jul 19, 2026
Merged

perf(player): avoid DomCrawler parse on every singular pageview#585
galbus merged 1 commit into
mainfrom
claude/amazing-hertz-2a6671

Conversation

@galbus

@galbus galbus commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What & why

Player::auto_prepend_player() is hooked to the_content at priority 1000000 for all post types, so it runs on every uncached singular front-end request. It called has_custom_player() — which constructs a full Symfony\Component\DomCrawler\Crawler (a libxml DOMDocument parse of the entire, unbounded post content) plus two XPath queries — before render_player()'s cheap Player::is_enabled() gate.

That parse ran on the hot path even when no player could ever render — i.e. virtually every request whose content lacks a player marker:

  • posts with no BeyondWords audio at all,
  • post types the plugin doesn't support,
  • unconfigured sites (no API key / project ID),
  • Player UI = Disabled,
  • Embed = None.

On large posts / high-traffic VIP sites this is measurable, avoidable render-time work on every uncached request, purely to detect a marker string that is almost always absent.

The fix

Two cheap short-circuits ahead of the parse:

  1. auto_prepend_player() now checks get_post() instanceof WP_Post && self::is_enabled( $post ) before calling has_custom_player().
  2. has_custom_player() guards the Crawler with a str_contains() pre-check for data-beyondwords-player and the SDK URL, so the DOM parse only runs when a marker is plausibly present.

Why it's behaviour-preserving

  • The reordering in auto_prepend_player() mirrors render_player()'s own early return (return '' when ! $post instanceof WP_Post || ! is_enabled()). Returning $content at the new gate is therefore exactly equivalent to prepending that ''. Verified across the full (is_singular, is_enabled, has_custom_player) truth table — identical output in every case.
  • I deliberately did not gate on Base::check/renderer eligibility (the "ideally" suggestion in the report): render_player() still fires the beyondwords_player_html filter with empty HTML when a post is enabled but no renderer matches, and short-circuiting there would silently drop that filter contract. The substring guard already makes that path cheap, so is_enabled is the correct gate.
  • The substring guard is a sound pre-filter: both XPath queries require a literal data-beyondwords-player attribute name or the SDK URL in a src to appear in the source, so their absence guarantees no match.
  • is_enabled() bottoms out in cached get_post_meta() / get_option() reads — far cheaper than the DOM parse it now front-runs.

Tests

  • PHPCS (WordPress-VIP-Go): clean, no phpcs:ignore added.
  • PHPUnit PlayerTest: OK (46 tests, 101 assertions). Added 3 regression cases:
    • the previously-untested SDK-<script> detection path,
    • a URL-in-text-only case (guard passes, XPath still returns false — proves the guard is a pre-filter, not a replacement),
    • the disabled-player short-circuit in auto_prepend_player().
  • Cypress suite not run (per project convention — no front-end player behaviour changed).

Reviewer notes

  • str_contains requires PHP ≥ 8.0, which the plugin already mandates (and already uses elsewhere, e.g. class-sync.php, class-client.php).
  • No new input/output, remote calls, or nonce/capability surface — these are pure read-side short-circuits, so no new VIP sanitisation/escaping obligations.

🤖 Generated with Claude Code

auto_prepend_player() is hooked to the_content at priority 1000000 for
all post types, so it runs on every uncached singular front-end request.
It called has_custom_player() -- which builds a full Symfony DomCrawler
(a libxml parse of the entire post content) plus two XPath queries --
before render_player()'s cheap Player::is_enabled() gate. That parse ran
even when no player could ever render: unconfigured sites, Player UI =
Disabled, Embed = None, unsupported post types, or posts with no audio.

Add two cheap short-circuits ahead of the parse:

1. auto_prepend_player() now checks get_post() instanceof WP_Post &&
   is_enabled( $post ) before calling has_custom_player(). This mirrors
   render_player()'s own early return, so returning $content here is
   exactly equivalent to prepending its '' -- verified behaviour-
   preserving across the full (is_singular, is_enabled,
   has_custom_player) truth table. It intentionally does NOT gate on
   Base::check, since render_player() still fires the
   beyondwords_player_html filter (with empty HTML) when enabled but no
   renderer matches.

2. has_custom_player() guards the DomCrawler with a str_contains()
   pre-check for the 'data-beyondwords-player' attribute and the SDK
   URL. Both XPath queries require one of those literal substrings in
   the markup, so their absence guarantees no match and we bail without
   parsing.

is_enabled() bottoms out in cached get_post_meta()/get_option() reads,
so the new gate is far cheaper than the DOM parse it replaces on the hot
path. No phpcs:ignore added; passes WordPress-VIP-Go.

Add regression tests: the previously-untested SDK-<script> detection
path, a URL-in-text-only case (guard passes, XPath still returns false),
and the disabled-player short-circuit in auto_prepend_player.

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

Copy link
Copy Markdown

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.


🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

@galbus
galbus marked this pull request as ready for review July 19, 2026 07:08
@galbus
galbus requested a review from gouravkhunger July 19, 2026 07:09
@galbus
galbus merged commit 5c5ac1f into main Jul 19, 2026
12 checks passed
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.

2 participants