Skip to content

[Performance] Avoid a redundant existence check on every theme file read - #8944

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
performance-maintenance-39180710266
Draft

github-actions[bot] wants to merge 1 commit into
mainfrom
performance-maintenance-39180710266

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

readThemeFile checked that a file existed and then read it, so every file that
does exist — which is nearly all of them — paid for two filesystem round-trips
instead of one. It runs once per theme file when mounting the theme file system,
so the cost scales with theme size on theme push, theme pull, and theme dev
startup.

WHAT is this pull request doing?

Read the file directly and treat an ENOENT/ENOTDIR failure as the "file
doesn't exist" case, which removes the extra fileExists call from the common
path. The returned value, the undefined result for a missing file, and the
debug log are unchanged; read errors that are not a missing file still propagate
rather than being swallowed as undefined.

Measured on a 1000-file theme, reading every file (12 iterations, 3 rounds, after
warm-up):

before after
round 1 133.5ms 112.3ms
round 2 129.5ms 114.2ms
round 3 111.8ms 98.4ms

A consistent 12–16% reduction in the read phase; the saving grows with file count.

How to manually test your changes?

shopify theme pull --store <store> --path <theme-dir>
shopify theme push --store <store> --path <theme-dir>
shopify theme dev --store <store> --path <theme-dir>

Pulling into a directory where some files are absent should still download them,
and a theme with no local copy of a file should behave exactly as before.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

readThemeFile stat'd each file before reading it, so every file that does
exist paid for two filesystem round-trips instead of one. Detect the missing
file from the failed read instead.

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

This branch has not been deployed

No deployments
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.

0 participants