perf(safe-js): reuse unchanged intrinsic retained roots - #717
Closed
kamilio wants to merge 1 commit into
Closed
Conversation
Cache only collected root references for groups backed entirely by tracked property tables. Successful property/prototype changes and baseline completion invalidate the cache; untracked and restored tables retain conservative scans. Nested values are still recursively measured on every reconciliation. Validation: 248 focused accounting/snapshot tests; maintained 24-workspace agent-harness build closure; unchanged five-second Node20 coverage-demo replay passed in 1.29s. Six new regressions cover allocation reuse, nested budget failure, descriptor/prototype changes, rejected mutations, restored aliases, and retained callback snapshots. Focused test typing and ESLint gate this commit. ### Tests Added | Test | Verifies | | --- | --- | | reuses the collected roots while still rejecting nested growth above the memory limit | Reference reuse preserves recursive budget rejection | | refreshes collected roots after define, delete, accessors, prototype changes and baseline completion | Successful mutations invalidate root collection | | remeasures mutations through an untracked restored property-table alias | Restored aliases keep conservative scanning | | keeps committed roots after failed definitions and deletions | Rejected mutations preserve committed state | | keeps both budgets current and ignores rejected prototype mutations | Shared objects invalidate each live budget correctly | | keeps root snapshots stable when a retained callback mutates the next collection | Callback mutation affects the next capture without changing prior snapshots |
Member
Author
|
Cherry-picked retained intrinsic roots caching once as 440d4ef; avoided its duplicate in #715. Verified on remote main at 4da2a66 after local validation and final installed-package qualification. Closing after cherry-pick/reconciliation rather than merging obsolete branch history. Release is in progress: https://github.com/poe-platform/poe-code/actions/runs/34500155547 ; scoped packages: https://github.com/poe-platform/poe-code/actions/runs/34500155125 . |
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
1 participant
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.
SafeJS repeatedly rebuilds the same intrinsic root-reference lists during memory reconciliation, including every replay segment. This change reuses those lists while every object in the group has a tracked property table and nothing has changed. Nested values are still recursively measured on every reconciliation; memory, step, and replay deadlines are unchanged.
Successful property/prototype mutations and baseline completion invalidate the lists. Restored or otherwise untracked tables retain the conservative scan. Callback snapshots remain stable and releasing a budget removes its retained source.
Validation:
This is a separate prerequisite for the replay timeout observed in PR715 and PR716. It is not included in the frozen safe-bash536 consumer build and does not address the independent tiny-MCP compiler-test timeout.
Hosted CI at this head
Node22 and packed public API checks passed. The shared Node20 job still failed the original five-second coverage-demo replay test, so the local improvement does not establish that the hosted deadline is resolved. The same job also reported the tiny-MCP NodeNext DOM deadline and the browser-fixture/Array.fromAsync compatibility failures owned by separate prerequisites. The provider E2E and external review automation failed their existing model/setup paths. No deadlines or accounting checks were relaxed, and no paid rerun was requested. This PR remains a draft while those failures are addressed.
Saved local verification evidence
This non-UI diagnostic was rendered from saved local logs for head
344ddf40439da30819085a58d8bc94a8f301bda7and captured with Playwright CLI. It records the reference-reuse RED case, 248 passing accounting/snapshot tests, the original focused replay test passing in 1.29 seconds under its unchanged five-second deadline, and saved build/type/lint results. It is not a product interaction or a fresh benchmark run; local results do not establish hosted CI timing.