Skip to content

Commit

Permalink
Memory leak: add rotatedPHP to kill and recreate PHP instances after …
Browse files Browse the repository at this point in the history
…a certain number of requests (#990)

Swaps the internal PHP runtime for a fresh one after a certain number of
requests are handled.

## Rationale

PHP and PHP extension have a memory leak. Each request leaves the memory
a bit more fragmented and with a bit less available space than before.
Eventually, new allocations start failing.

Rotating the PHP instance may seem like a workaround, but it's actually
what PHP-FPM does natively:

Implementing this solution in core enables all downstream consumers of
the PHP WASM package, like `wp-now`, to benefit.

## How it works

Adds a `rotatePHP` function that keeps track of the `BasePHP.run()`
calls and replaces the Emscripten runtime with a new one after a certain
number of calls.

Example:

```ts
const loadRuntime = async () => {
	return await NodePHP.loadRuntime("8.0");
}

const php = await rotatedPHP({
	php: new NodePHP(await loadRuntime(), {
		documentRoot: '/test-root',
	}),
	recreateRuntime: loadRuntime,
	maxRequests: 50
});

// After 50 request() calls, the PHP runtime will be discarded and a new one will be created.
// It all happens internally. The caller doesn't know anything about this.
```

I started by porting the "Process Pool" idea ([see the
branch](https://github.com/WordPress/wordpress-playground/compare/harvested-memory-pool?expand=1))
from WordPress/playground-tools#110, but I
realized that:

* The logic was complicated
* We only need to manage a single PHP instance, not a pool of them
* Worker thread is built to a single PHP instance and not architected to
handle a pool

So I decided to simplify the logic and just manage a single PHP
instance.

## Other changes

* Implements `BasePHP.hotSwapPHPRuntime()` that opens the door to
switching PHP versions dynamically and without a full page reload.
* Makes the `BasePHP.semaphore` property public to enable conflict-free
runtime swaps
* Adds the `runtime.initialized` and `runtime.beforedestroy` events for
OPFS handlers to re-bind on runtime swap.
* Tiny unrelated change: Migrates the OPFS handler from monkey-patching
the `php.run` method to relying on event listeners. The `rotatePHP`
function went through a similar journey and I updated that other code
path while I was on it.


Supersedes WordPress/playground-tools#110
  • Loading branch information
adamziel authored Feb 2, 2024
1 parent 90b96e6 commit 07fd316
Show file tree
Hide file tree
Showing 13 changed files with 547 additions and 167 deletions.
112 changes: 64 additions & 48 deletions packages/php-wasm/fs-journal/src/lib/fs-journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,61 +116,77 @@ export function journalFSEvents(
fsRoot: string,
onEntry: (entry: FilesystemOperation) => void = () => {}
) {
fsRoot = normalizePath(fsRoot);
const FS = php[__private__dont__use].FS;
const FSHooks = createFSHooks(FS, (entry: FilesystemOperation) => {
// Only journal entries inside the specified root directory.
if (entry.path.startsWith(fsRoot)) {
onEntry(entry);
} else if (
entry.operation === 'RENAME' &&
entry.toPath.startsWith(fsRoot)
) {
for (const op of recordExistingPath(
php,
entry.path,
entry.toPath
)) {
onEntry(op);
function bindToCurrentRuntime() {
fsRoot = normalizePath(fsRoot);
const FS = php[__private__dont__use].FS;
const FSHooks = createFSHooks(FS, (entry: FilesystemOperation) => {
// Only journal entries inside the specified root directory.
if (entry.path.startsWith(fsRoot)) {
onEntry(entry);
} else if (
entry.operation === 'RENAME' &&
entry.toPath.startsWith(fsRoot)
) {
for (const op of recordExistingPath(
php,
entry.path,
entry.toPath
)) {
onEntry(op);
}
}
}
});
});

/**
* Override the original FS functions with ones running the hooks.
* We could use a Proxy object here if the Emscripten JavaScript module
* did not use hard-coded references to the FS object.
*/
const originalFunctions: Record<string, Function> = {};
for (const [name] of Object.entries(FSHooks)) {
originalFunctions[name] = FS[name];
}
/**
* Override the original FS functions with ones running the hooks.
* We could use a Proxy object here if the Emscripten JavaScript module
* did not use hard-coded references to the FS object.
*/
const originalFunctions: Record<string, Function> = {};
for (const [name] of Object.entries(FSHooks)) {
originalFunctions[name] = FS[name];
}

// eslint-disable-next-line no-inner-declarations
function bind() {
for (const [name, hook] of Object.entries(FSHooks)) {
FS[name] = function (...args: any[]) {
// @ts-ignore
hook(...args);
return originalFunctions[name].apply(this, args);
};
// eslint-disable-next-line no-inner-declarations
function bind() {
for (const [name, hook] of Object.entries(FSHooks)) {
FS[name] = function (...args: any[]) {
// @ts-ignore
hook(...args);
return originalFunctions[name].apply(this, args);
};
}
}
}
// eslint-disable-next-line no-inner-declarations
function unbind() {
// Restore the original FS functions.
for (const [name, fn] of Object.entries(originalFunctions)) {
php[__private__dont__use].FS[name] = fn;
// eslint-disable-next-line no-inner-declarations
function unbind() {
// Restore the original FS functions.
for (const [name, fn] of Object.entries(originalFunctions)) {
php[__private__dont__use].FS[name] = fn;
}
}

php[__private__dont__use].journal = {
bind,
unbind,
};
bind();
}
php.addEventListener('runtime.initialized', bindToCurrentRuntime);
if (php[__private__dont__use]) {
bindToCurrentRuntime();
}

php[__private__dont__use].journal = {
bind,
unbind,
};
function unbindFromOldRuntime() {
php[__private__dont__use].journal.unbind();
delete php[__private__dont__use].journal;
}
php.addEventListener('runtime.beforedestroy', unbindFromOldRuntime);

bind();
return unbind;
return function unbind() {
php.removeEventListener('runtime.initialized', bindToCurrentRuntime);
php.removeEventListener('runtime.beforedestroy', unbindFromOldRuntime);
return php[__private__dont__use].journal.unbind();
};
}

const createFSHooks = (
Expand Down Expand Up @@ -261,7 +277,7 @@ const createFSHooks = (
*/
export function replayFSJournal(php: BasePHP, entries: FilesystemOperation[]) {
// We need to restore the original functions to the FS object
// before proceeding, or each replayer FS operation will be journaled.
// before proceeding, or each replayed FS operation will be journaled.
//
// Unfortunately we can't just call the non-journaling versions directly,
// because they call other low-level FS functions like `FS.mkdir()`
Expand Down
54 changes: 19 additions & 35 deletions packages/php-wasm/node/src/lib/node-php.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
rethrowFileSystemError,
__private__dont__use,
isExitCodeZero,
DataModule,
} from '@php-wasm/universal';

import { lstatSync, readdirSync } from 'node:fs';
Expand All @@ -17,7 +16,6 @@ import { withNetworking } from './networking/with-networking.js';
export interface PHPLoaderOptions {
emscriptenOptions?: EmscriptenOptions;
requestHandler?: PHPRequestHandlerConfiguration;
dataModules?: Array<DataModule | Promise<DataModule>>;
}

export type MountSettings = {
Expand All @@ -44,20 +42,10 @@ export class NodePHP extends BasePHP {
phpVersion: SupportedPHPVersion,
options: PHPLoaderOptions = {}
) {
return await NodePHP.loadSync(phpVersion, {
...options,
emscriptenOptions: {
/**
* Emscripten default behavior is to kill the process when
* the WASM program calls `exit()`. We want to throw an
* exception instead.
*/
quit: function (code, error) {
throw error;
},
...(options.emscriptenOptions || {}),
},
}).phpReady;
return new NodePHP(
await NodePHP.loadRuntime(phpVersion, options),
options.requestHandler
);
}

/**
Expand All @@ -67,29 +55,25 @@ export class NodePHP extends BasePHP {
*
* @see load
*/
static loadSync(
static async loadRuntime(
phpVersion: SupportedPHPVersion,
options: PHPLoaderOptions = {}
) {
/**
* Keep any changes to the signature of this method in sync with the
* `PHP.load` method in the @php-wasm/node package.
*/
const php = new NodePHP(undefined, options.requestHandler);

const doLoad = async () => {
const runtimeId = await loadPHPRuntime(
await getPHPLoaderModule(phpVersion),
await withNetworking(options.emscriptenOptions || {})
);
php.initializeRuntime(runtimeId);
};
const asyncData = doLoad();

return {
php,
phpReady: asyncData.then(() => php),
const emscriptenOptions: EmscriptenOptions = {
/**
* Emscripten default behavior is to kill the process when
* the WASM program calls `exit()`. We want to throw an
* exception instead.
*/
quit: function (code, error) {
throw error;
},
...(options.emscriptenOptions || {}),
};
return await loadPHPRuntime(
await getPHPLoaderModule(phpVersion),
await withNetworking(emscriptenOptions)
);
}

/**
Expand Down
Loading

0 comments on commit 07fd316

Please sign in to comment.