Fix progressive performance degradation in playground.run()#3235
Draft
Fix progressive performance degradation in playground.run()#3235
Conversation
Each property access on a Comlink proxy created a new proxy object registered with FinalizationRegistry. Over many calls, these proxies accumulated and caused progressive performance degradation. This adds a WeakMap-based cache that reuses existing proxies for the same endpoint and path combination. The cache is automatically cleaned up when the endpoint is garbage collected, and explicitly cleared when the proxy is released.
The streamToText() and streamToBytes() functions didn't properly release reader locks in all code paths. This adds try/finally blocks with reader.releaseLock() to ensure proper cleanup regardless of how the function exits.
When using SinglePHPInstanceManager, the same PHP instance is reused across multiple run() calls. Previously, registerWorkerListeners() would add new event listeners on every run(), causing O(n) listener accumulation where each event fired N times for N calls. This adds a WeakSet to track PHP instances that have already had listeners registered, preventing duplicate registration and eliminating the progressive performance degradation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for the change, related issues
Repeated calls to
playground.run()exhibited progressive performance degradation. The first batch of calls would execute in ~1ms, but after many calls the execution time would increase to 10-11ms — an 11.76x slowdown.This affected browser-based usage where the same PHP instance is reused across requests via
SinglePHPInstanceManager.Implementation details
Three sources of resource accumulation were identified and fixed:
1. Comlink proxy accumulation (
comlink-sync.ts)Each property access on a Comlink proxy created a new proxy object registered with
FinalizationRegistry. These proxies accumulated over time. Fixed by adding aWeakMap-based cache that reuses existing proxies for the same endpoint and path combination.2. Stream reader cleanup (
php-response.ts)The
streamToText()andstreamToBytes()functions didn't properly release reader locks in all code paths. Fixed by addingtry/finallyblocks withreader.releaseLock().3. Worker listener accumulation (
php-worker.ts)When using
SinglePHPInstanceManager, the same PHP instance is reused across multiplerun()calls. Previously,registerWorkerListeners()added new event listeners on every call, causing O(n) listener accumulation where each event fired N times for N calls. Fixed by tracking registered instances in aWeakSetto prevent duplicate registration.Testing Instructions (or ideally a Blueprint)
Run this in the browser console on the Playground website:
Before: Ratio ~11.76x (gets slower)
After: Ratio ~0.7x (gets faster due to JIT — no degradation)