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 Native PHP worker pool on all platforms 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants