Skip to content

Commit ddc9765

Browse files
bcotrimfredrikekelundgithub-advanced-security[bot]
authored
Native PHP worker pool on all platforms (#3602)
## Related issues <!-- Link a related issue to this PR. If the PR does not immediately resolve the issue, for example, it requires a separate deployment to production, avoid using the "Fixes" keyword and use "Related to" instead. --> - Fixes # ## How AI was used in this PR <!-- Help reviewers understand what to look for and verify that you've reviewed the code yourself. --> ## 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 <!-- Add as many details as possible to help others reproduce the issue and test the fix. "Before / After" screenshots can also be very helpful when the change is visual. --> CI should pass ## Pre-merge Checklist <!-- Complete applicable items on this checklist **before** merging into trunk. Inapplicable items can be left unchecked. Both the PR author and reviewer are responsible for ensuring the checklist is completed. --> - [ ] Have you checked for TypeScript, React or other console errors? --------- Co-authored-by: Fredrik Rombach Ekelund <fredrik.rombach.ekelund@automattic.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>
1 parent 13a78be commit ddc9765

16 files changed

Lines changed: 1069 additions & 563 deletions

File tree

‎.buildkite/commands/run-e2e-tests.sh‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,15 @@ if [ "$PLATFORM" = "linux" ]; then
152152
# right-edge content (e.g. the preferences Save button) below the fold
153153
# for the split-pane settings layout. 1920x1080 matches a typical
154154
# desktop and avoids relying on scroll-into-view.
155+
inner_exit=0
155156
xvfb-run -a -s "-screen 0 1920x1080x24" \
156-
npx playwright test --max-failures=1 --output=/tmp/test-results
157+
npx playwright test --max-failures=1 --output=/tmp/test-results || inner_exit=$?
158+
# On failure, collect daemon logs into /tmp/test-results (copied to the
159+
# artifact dir below). $HOME here is the node user that ran the tests.
160+
if [ "$inner_exit" -ne 0 ] && [ -d "$HOME/.studio/daemon/logs" ]; then
161+
cp -r "$HOME/.studio/daemon/logs" /tmp/test-results/daemon-logs || true
162+
fi
163+
exit "$inner_exit"
157164
' || test_exit=$?
158165

159166
if [ -d /tmp/test-results ]; then
@@ -168,5 +175,15 @@ else
168175
npx playwright install
169176

170177
echo 'Running Playwright tests...'
171-
npx playwright test
178+
# Capture the exit code so a failure doesn't trip `set -e` before we collect
179+
# the daemon logs (~/.studio/daemon/logs) for artifact upload.
180+
test_exit=0
181+
npx playwright test || test_exit=$?
182+
183+
if [ "$test_exit" -ne 0 ] && [ -d "$HOME/.studio/daemon/logs" ]; then
184+
mkdir -p test-results/daemon-logs
185+
cp -r "$HOME/.studio/daemon/logs/." test-results/daemon-logs/ || true
186+
fi
187+
188+
exit "$test_exit"
172189
fi

‎.buildkite/pipeline.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ e2e_config: &e2e_config
1212
- test-results/**/*.zip
1313
- test-results/**/*.png
1414
- test-results/**/*error-context.md
15+
- test-results/daemon-logs/**/*.log
1516
plugins: [$CI_TOOLKIT_PLUGIN, $NVM_PLUGIN]
1617
agents:
1718
queue: "{{matrix.platform}}"
@@ -101,6 +102,10 @@ steps:
101102
STUDIO_RUNTIME: native-php
102103
if: (build.branch == 'trunk' || build.tag =~ /^v[0-9]+/ || !build.pull_request.draft) && build.pull_request.labels includes 'native-php'
103104

105+
# Linux E2E tests run on the shared `default` queue inside a Debian Node
106+
# container — the same pattern the Linux build/unit-test steps use. Kept as
107+
# a separate step (rather than another matrix entry on *e2e_config) because
108+
# Mac and Windows share queue/plugin defaults that Linux doesn't.
104109
# Linux E2E tests run on the shared `default` queue inside a Debian Node
105110
# container — the same pattern the Linux build/unit-test steps use. Kept as
106111
# a separate step (rather than another matrix entry on *e2e_config) because
@@ -119,6 +124,7 @@ steps:
119124
- test-results/**/*.zip
120125
- test-results/**/*.png
121126
- test-results/**/*error-context.md
127+
- test-results/daemon-logs/**/*.log
122128
env:
123129
DEBUG: "pw:browser"
124130
# TEMP(rsm-2593): if: removed to force E2E on every push while iterating on Linux E2E setup. Revert before merge.
@@ -140,6 +146,7 @@ steps:
140146
- test-results/**/*.zip
141147
- test-results/**/*.png
142148
- test-results/**/*error-context.md
149+
- test-results/daemon-logs/**/*.log
143150
env:
144151
DEBUG: "pw:browser"
145152
STUDIO_RUNTIME: native-php

‎apps/cli/commands/wp.ts‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import { connectToDaemon, disconnectFromDaemon } from 'cli/lib/daemon-client';
1111
import { getPhpBinaryPath, getWpCliPharPath } from 'cli/lib/dependency-management/paths';
1212
import { ensurePhpBinaryAvailable } from 'cli/lib/dependency-management/php-binary';
1313
import { getSiteRuntime } from 'cli/lib/feature-flags';
14-
import { getDefaultPhpArgs } from 'cli/lib/native-php';
14+
import { getDefaultPhpArgs } from 'cli/lib/native-php/config';
15+
import { DETACH_FOR_GROUP_KILL, reapPhpTreeOnInterrupt } from 'cli/lib/native-php/php-process';
1516
import { runWpCliCommand, runGlobalWpCliCommand, WpCliResponse } from 'cli/lib/run-wp-cli-command';
1617
import { validatePhpVersion } from 'cli/lib/utils';
1718
import { isServerRunning, sendWpCliCommand } from 'cli/lib/wordpress-server-manager';
@@ -58,18 +59,28 @@ async function runNativePhpWpCliCommand( site: SiteData, args: string[] ): Promi
5859
{
5960
cwd: site.path,
6061
stdio: 'inherit',
62+
detached: DETACH_FOR_GROUP_KILL,
6163
}
6264
);
6365

64-
const { code, signal } = await new Promise< {
65-
code: number | null;
66-
signal: NodeJS.Signals | null;
67-
} >( ( resolve, reject ) => {
68-
child.once( 'error', reject );
69-
child.once( 'exit', ( exitCode, exitSignal ) =>
70-
resolve( { code: exitCode, signal: exitSignal } )
71-
);
72-
} );
66+
// Reap php.exe and any subprocess it spawned if this command is interrupted before the child exits.
67+
const removeReaper = reapPhpTreeOnInterrupt( child );
68+
69+
let code: number | null;
70+
let signal: NodeJS.Signals | null;
71+
try {
72+
( { code, signal } = await new Promise< {
73+
code: number | null;
74+
signal: NodeJS.Signals | null;
75+
} >( ( resolve, reject ) => {
76+
child.once( 'error', reject );
77+
child.once( 'exit', ( exitCode, exitSignal ) =>
78+
resolve( { code: exitCode, signal: exitSignal } )
79+
);
80+
} ) );
81+
} finally {
82+
removeReaper();
83+
}
7384

7485
if ( signal ) {
7586
process.kill( process.pid, signal );

‎apps/cli/lib/dependency-management/php-binary.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
type PhpBinaryDownloadInfo,
1212
type NativePhpSupportedVersion,
1313
} from '@studio/common/lib/php-binary-metadata';
14-
import { ensureNativePhpIniFiles } from '../native-php';
14+
import { ensureNativePhpIniFiles } from 'cli/lib/native-php/config';
1515
import { getPhpBinaryPath } from './paths';
1616
import type { SupportedPHPVersion } from '@studio/common/types/php-versions';
1717

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { getBlueprintsPharPath } from 'cli/lib/dependency-management/paths';
4+
import { runPhpCommand } from './php-process';
5+
import type { NativePhpSupportedVersion } from '@studio/common/lib/php-binary-metadata';
6+
import type { ServerConfig } from 'cli/lib/types/wordpress-server-ipc';
7+
8+
export async function runBlueprint(
9+
config: ServerConfig,
10+
blueprint: NonNullable< ServerConfig[ 'blueprint' ] >,
11+
phpVersion: NativePhpSupportedVersion,
12+
signal: AbortSignal
13+
): Promise< void > {
14+
// blueprints.phar accepts local paths only; remote URIs need the Playground runtime.
15+
if ( blueprint.uri.startsWith( 'http://' ) || blueprint.uri.startsWith( 'https://' ) ) {
16+
throw new Error(
17+
`Remote blueprint URIs are not supported by the native PHP runtime: ${ blueprint.uri }`
18+
);
19+
}
20+
21+
const enableDebugLog = config.enableDebugLog ?? false;
22+
const enableDebugDisplay = config.enableDebugDisplay ?? false;
23+
const defaultConstants: Record< string, boolean | string > = {
24+
// The SQLite driver requires a non-empty DB_NAME at runtime.
25+
DB_NAME: 'wordpress',
26+
WP_DEBUG: enableDebugLog || enableDebugDisplay,
27+
WP_DEBUG_LOG: enableDebugLog,
28+
WP_DEBUG_DISPLAY: enableDebugDisplay,
29+
};
30+
31+
blueprint.contents.constants = {
32+
...blueprint.contents.constants,
33+
...defaultConstants,
34+
};
35+
// Native PHP selects PHP and installs WordPress before Blueprint execution.
36+
// Passing preferredVersions makes blueprints.phar validate versions it does not manage here.
37+
delete blueprint.contents.preferredVersions;
38+
39+
const blueprintDir = path.dirname( blueprint.uri );
40+
const tmpPath = path.join( blueprintDir, `studio-blueprint-${ config.siteId }.json` );
41+
await fs.promises.writeFile( tmpPath, JSON.stringify( blueprint.contents ) );
42+
43+
// blueprints.phar detects SQLite under plugins, while Studio installs it under mu-plugins.
44+
const muPluginsSqlite = path.join(
45+
config.sitePath,
46+
'wp-content',
47+
'mu-plugins',
48+
'sqlite-database-integration'
49+
);
50+
const pluginsSqlite = path.join(
51+
config.sitePath,
52+
'wp-content',
53+
'plugins',
54+
'sqlite-database-integration'
55+
);
56+
const needsSymlink = fs.existsSync( muPluginsSqlite ) && ! fs.existsSync( pluginsSqlite );
57+
let symlinkIno: number | undefined;
58+
if ( needsSymlink ) {
59+
fs.symlinkSync( muPluginsSqlite, pluginsSqlite, 'junction' );
60+
// Remove only the entry created here, not unrelated content that replaced it.
61+
symlinkIno = fs.statSync( pluginsSqlite ).ino;
62+
}
63+
64+
try {
65+
await runPhpCommand(
66+
[
67+
getBlueprintsPharPath(),
68+
'exec',
69+
tmpPath,
70+
'--mode=apply-to-existing-site',
71+
`--site-path=${ config.sitePath }`,
72+
`--site-url=${ config.absoluteUrl ?? `http://localhost:${ config.port }` }`,
73+
'--db-engine=sqlite',
74+
],
75+
{ phpVersion, signal }
76+
);
77+
} finally {
78+
await fs.promises.unlink( tmpPath ).catch( () => {} );
79+
if ( needsSymlink ) {
80+
try {
81+
if ( fs.statSync( pluginsSqlite ).ino === symlinkIno ) {
82+
await fs.promises.rm( pluginsSqlite, { recursive: true, force: true } );
83+
}
84+
} catch {
85+
// Best effort - leaving the symlink behind is non-fatal.
86+
}
87+
}
88+
}
89+
}
Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import os from 'os';
44
import path from 'path';
55
import { NativePhpSupportedVersion } from '@studio/common/lib/php-binary-metadata';
66
import { writeFile } from 'atomically';
7-
import { getPhpBinaryPath } from './dependency-management/paths';
7+
import semver from 'semver';
8+
import { getPhpBinaryPath } from '../dependency-management/paths';
89

910
// Disabled by default to shrink the attack surface available to PHP code
1011
// running inside a Studio site. Each entry falls into one of:
@@ -151,9 +152,14 @@ export function getNativePhpIniContents( phpVersion: NativePhpSupportedVersion )
151152
if ( process.platform === 'win32' ) {
152153
directives.push(
153154
`extension_dir="${ toPhpIniPath( getExtensionDir( phpVersion ) ) }"`,
154-
'zend_extension=opcache',
155155
...WINDOWS_PHP_EXTENSIONS.map( ( extension ) => `extension=${ extension }` )
156156
);
157+
158+
const coercedVersion = semver.coerce( phpVersion );
159+
// As of PHP 8.5, the OPcache extension is always bundled with PHP
160+
if ( coercedVersion && semver.lt( coercedVersion, '8.5.0' ) ) {
161+
directives.push( 'zend_extension=opcache' );
162+
}
157163
}
158164

159165
return `${ directives.join( os.EOL ) }${ os.EOL }`;

0 commit comments

Comments
 (0)