[9.4] fix(zod): adopt upstream lazy-bind memory optimization (colinhacks/zod#5897) (#266343)#266411
Closed
kibanamachine wants to merge 1 commit intoelastic:9.4from
Closed
[9.4] fix(zod): adopt upstream lazy-bind memory optimization (colinhacks/zod#5897) (#266343)#266411kibanamachine wants to merge 1 commit intoelastic:9.4from
kibanamachine wants to merge 1 commit intoelastic:9.4from
Conversation
2 tasks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…d#5897) (elastic#266343) ## Summary Adopts the memory optimization implemented by Colin McDonnell in [colinhacks/zod#5897](colinhacks/zod#5897) as a `patch-package` patch, ahead of the next official zod release. Also fixes [colinhacks/zod#5760](colinhacks/zod#5760). ### Background The previous patch (elastic#263121) introduced a **shadow-proto** architecture that shared builder methods on a hidden prototype layer. While it reduced per-schema heap cost by ~80%, it introduced a breaking change: **detached method calls** stopped working. ```ts const opt = schema.optional; // extracts method reference opt(); // ❌ previously threw — `this` was undefined ``` The root cause was that `_initProto` used `Object.assign` to copy methods as plain value properties onto the prototype. Detaching such a property loses the `this` context. ### This PR Replaces `_initProto` with Colin's `_installLazyMethods` approach from [colinhacks/zod#5897](colinhacks/zod#5897): ```js function _installLazyMethods(inst, group, methods) { // ... one-time setup per (proto, group) ... for (const key in methods) { const fn = methods[key]; Object.defineProperty(proto, key, { configurable: true, enumerable: false, get() { const bound = fn.bind(this); // Cache on the instance — subsequent accesses skip the getter Object.defineProperty(this, key, { configurable: true, writable: true, enumerable: true, value: bound, }); return bound; }, set(v) { Object.defineProperty(this, key, { configurable: true, writable: true, enumerable: true, value: v, }); }, }); } } ``` **How it works:** - Builder methods live as non-enumerable getters on each concrete schema's prototype - On first access per instance, the getter allocates `fn.bind(this)` and caches it as an own property — subsequent accesses skip the getter entirely - Detached calls (`const m = schema.optional; m()`) work because `m` is a bound function with `this` already resolved - Parse-family methods (`parse`, `safeParse`, etc.) stay as eager per-instance closures — they're the hot path and most-detached methods **Memory characteristics are unchanged:** builder methods are not own properties until first accessed, so V8 keeps schema instances in fast-property mode. Per Colin's numbers: -40% heap per instance, -25% construction time vs Zod 4.3.6 baseline. ## Why `patch-package` Upstream zod CI has been broken for months and there is no scheduled release. `patch-package` allows us to ship the fix now. Once a new zod version including [colinhacks/zod#5897](colinhacks/zod#5897) is released, this patch can be dropped. ## Changes - `patches/zod+4.3.6.patch` — replaces the previous shadow-proto patch with Colin's `_installLazyMethods` approach (only touches `v4/classic/schemas.cjs` and `v4/classic/schemas.js`) - `src/platform/packages/shared/kbn-zod/v4/detached_methods.test.ts` — behavioural unit tests proving detached calls work and that the lazy-cache correctly returns the same bound function on repeated access ## Test plan - [x] `node scripts/jest src/platform/packages/shared/kbn-zod/v4/detached_methods.test.ts` — 5/5 pass - [ ] Full CI green --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit a9ecabf)
kibanamachine
force-pushed
the
backport/9.4/pr-266343
branch
from
9cb8bba to
667cd64
Compare
Contributor
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
Member
|
backported manually through #266401 |
Contributor
Author
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
Page load bundle
cc @gsoldevila |
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
3 participants
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.
Backport
This will backport the following commits from
mainto9.4:Questions ?
Please refer to the Backport tool documentation