Skip to content

Native PHP worker pool on all platforms - #3602

Merged
fredrikekelund merged 27 commits into
trunkfrom
add-native-php-worker-pool-poc
Jun 2, 2026
Merged

Native PHP worker pool on all platforms#3602
fredrikekelund merged 27 commits into
trunkfrom
add-native-php-worker-pool-poc

Conversation

@bcotrim

@bcotrim bcotrim commented May 22, 2026

Copy link
Copy Markdown
Contributor

Related issues

  • Fixes #

How AI was used in this PR

Proposed Changes

Using PHP_CLI_SERVER_WORKERS for concurrency is only supported on macOS and Linux. We want concurrency on Windows, too. This PR accomplishes that by implementing a Node.js HTTP load balancer that proxies requests to a fixed pool of 4 php -S processes. This works surprisingly well in our testing.

In our benchmarks, this solution yields ~30% performance improvement on Windows compared to Playground.

Testing Instructions

CI should pass

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@bcotrim bcotrim self-assigned this May 22, 2026
Comment thread apps/cli/php-server-child.ts Fixed
Comment thread apps/cli/php-server-child.ts Fixed
bcotrim and others added 3 commits May 28, 2026 18:58
… through a stack trace'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@fredrikekelund fredrikekelund changed the title Add native PHP worker pool POC May 29, 2026
@fredrikekelund
fredrikekelund marked this pull request as ready for review May 29, 2026 16:00
@wpmobilebot

wpmobilebot commented May 29, 2026

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing c829764 vs trunk

app-size

Metric trunk c829764 Diff Change
App Size (Mac) 1329.72 MB 1329.73 MB +0.01 MB ⚪ 0.0%

site-editor

Metric trunk c829764 Diff Change
load 1649 ms 1659 ms +10 ms ⚪ 0.0%

site-startup

Metric trunk c829764 Diff Change
siteCreation 9013 ms 8993 ms 20 ms ⚪ 0.0%
siteStartup 4402 ms 4383 ms 19 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@bcotrim
bcotrim requested a review from fredrikekelund May 31, 2026 07:33

@fredrikekelund fredrikekelund 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.

LGTM 👍 @bcotrim and I worked on this side by side at our meetup last week. Benchmark-wise, we concluded that there's no noticeable drawback to this approach compared to PHP_CLI_SERVER_WORKERS, so we're going with this "poor man's php-fpm" approach on all platforms

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.

The changes in this file fix RSM-3965

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.

The changes in this file and in .buildkite/pipeline.yml serve to collect all site logs when an E2E test fails. Site logs are typically forwarded to the main process anyway, but this seems like a sensible thing to keep as the default. It will only make debugging easier

@fredrikekelund

Copy link
Copy Markdown
Contributor

The Linux E2E breakage seems to be CI specific and it exists on trunk, too. I'm proceeding to merge this PR

@fredrikekelund
fredrikekelund merged commit ddc9765 into trunk Jun 2, 2026
12 of 13 checks passed
@fredrikekelund
fredrikekelund deleted the add-native-php-worker-pool-poc branch June 2, 2026 08:13
bcotrim added a commit that referenced this pull request Jun 12, 2026
## Related issues

- Closes STU-1537 (completes the part deferred from #3125)

## How AI was used in this PR

Claude Code reconstructed the April context (#3125's deferred section,
this PR's original debug instrumentation, and the language-pack copy
races later documented in #3492), re-applied the bundle-read change
cleanly on top of current trunk.

## Proposed Changes

Language packs were the last site dependency still copied from the
read-only CLI bundle into the writable `~/.studio/server-files/` cache
on every CLI invocation. The copy was pure overhead: language packs are
only ever read at site creation, to copy the matching locale's files
into the new site's `wp-content/languages/`, and they only change when
the app ships a new bundle.

This PR has site creation read language packs directly from the bundled
`wp-files/latest/languages/`, and drops the per-invocation copy step —
shaving the remaining recursive dir-walk (~1,400 files) off CLI startup.

Side benefits:

- The copy step needed lockfile serialization (#3492) because concurrent
CLI invocations raced on the shared cache (transient `ENOENT` warnings).
With no copy and no writable cache, that bug class is gone by
construction.
- Site creation now skips the language-pack directory scans entirely for
locales we don't bundle translations for (`WP_LOCALES`).
- The existing `04-cleanup-obsolete-server-files` migration also removes
the now-unused `~/.studio/server-files/language-packs/` (and its
lockfile) on first run after upgrade.

**On the April E2E failures that deferred this:** they never reproduced
locally — neither at the CLI level (3/3 ja-locale sites healthy,
WPLANG=ja, wp-admin 200) nor with the packaged-app Playwright suites
that failed in CI back then. The Playground worker-spawn stack has been
rewritten since (#3602), and the failure was never root-caused, so CI on
this PR is the deciding signal. If mac E2E reds again, this time it
fails on a clean branch with inspectable artifacts.

## Testing Instructions

1. `npm run download-language-packs` (plain `npm install` does not
populate `wp-files/latest/languages`), then `npm run cli:build`.
2. Set a non-English locale (e.g. `"locale": "ja"` in
`~/.studio/shared.json` or via Studio Settings) and create a site.
3. Verify the site's `wp-content/languages/` contains the locale's files
(core + `plugins/` + `themes/`), wp-admin loads in that language, and
Settings → General shows the locale selected under Site Language.
4. Verify `~/.studio/server-files/language-packs/` is deleted on first
CLI run after upgrade and is not recreated.
5. Covered by e2e `localization.test.ts` (locale site creation + WPLANG)
and `blueprints.test.ts` (blueprint provisioning).

Verified locally on macOS arm64: typecheck, eslint, full unit suite
(1963 tests), packaged-app e2e `localization.test.ts` 4/4 (twice) and
`blueprints.test.ts` 6/6.

## Pre-merge Checklist

- [x] Have you checked for TypeScript, React or other console errors?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants