Skip to content

perf(v4): prefix issue paths in place in the object JIT failure path - #6445

Merged
colinhacks merged 2 commits into
mainfrom
perf-jit-issue-prefix
Aug 20, 2026
Merged

perf(v4): prefix issue paths in place in the object JIT failure path#6445
colinhacks merged 2 commits into
mainfrom
perf-jit-issue-prefix

Conversation

@colinhacks

@colinhacks colinhacks commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Salvaged from #6319, which is closed — this is the half of it that costs zod/mini nothing.

The object JIT copied every issue to prefix a path onto it:

payload.issues = payload.issues.concat(id.issues.map(iss => ({
  ...iss, path: iss.path ? [k, ...iss.path] : [k]
})));

The interpreted paths — arrays, records, and the jitless object path — have always prefixed in place through util.prefixIssues. The JIT path was the odd one out, so it now emits a loop that writes iss.path and pushes. The three call sites emit identical code, so the template is built once, which takes 262 minified bytes off a classic object bundle.

Measured against #6443 with a paired A/B harness, two runs, on a noise floor of ±0.3% for this case:

case run 1 run 2
union-parse −12.3% −12.8%
suite total −1.5% −0.9%

Unions are where it shows up: every option before the matching one fails and hands its issues up.

Bundle: 0 bytes on all three zod/mini fixtures, which never bundle the object JIT, and +8 gzipped on classic zod-object. Memory is unchanged.

Prefixing in place is only safe on top of #6443, now merged as b63db248: the memoizer used to hand the same issue objects to every visitor of a shared node, so without that fix a DAG-shaped input reaching one invalid node twice comes out with both paths prefixed onto a single object.

Refs #6319.

Copilot AI lite review requested due to automatic review settings August 19, 2026 21:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — the object JIT's issue-path prefixing, plus the #6443 memoizer fix (89c487e7) this branch carries because main doesn't have it yet.

  • JIT prefixes in placeschemas.ts:2134 swaps the per-issue {...iss} copy and payload.issues.concat(...) for a loop that writes iss.path and pushes, which is what util.prefixIssues has always done for arrays, tuples, records, maps and the jitless object path.
  • One template, three emission sites — the loop body moves into prefixStr(id, k), shared by the optin+optout, required, and optin-only branches.
  • Memoizer keeps cached issues privatecloneIssues at memoizer.ts:31, applied on both the store side (:184, :188) and the hand-out side (:164).
  • Regression testcyclic-data.test.ts:622 parses a DAG whose single invalid node is reached twice, under both jitless settings.

The aliasing question is the whole risk here, and it holds up. Outside the memoizer I found no path where one $ZodRawIssue reaches two parent payloads: union options get a fresh payload each and handleUnionResults returns either final or nonaborted[0], never both; handlePipeResult and handleCodecTxResult forward the array identity only on the branch that doesn't also return left; the backward-direction canary allocates its own array at :299 and short-circuits the second pass when aborted. runChecks captures a length rather than the array, so nothing depended on the old concat replacing payload.issues. The emitted for re-reads ${id}.issues.length each iteration and would spin forever if handed ${id}.issues === payload.issues, but ${id} is always a fresh payload from :2131 and the parent's is never passed down. core/compile.ts needs no matching change — its fast path is boolean-only and falls back to the runtime to report issues.

Both halves of the memoizer fix are load-bearing, and so is the test: reverting the three memoizer.ts hunks to push(...hit.issues) / issues.slice() makes the new test fail under jitless: false and jitless: true, yielding [["right","left","name"], ["right","left","name"]]. Full suite is green — 7391 passed including the compile-mode project, with only the three packages/treeshake tests failing because this checkout isn't built.

I did not re-measure the perf or bundle numbers; a GitHub Actions runner is exactly the loaded machine that makes those worthless.

ℹ️ The stated merge order no longer matches the branch

The body says to merge #6443 first, but the base has been retargeted to main and 89c487e7 now rides on this branch, so this PR is self-contained as it stands. Landing #6443 first means rebasing here; landing this first leaves #6443 with an empty diff.

Technical details
# Merge order for the #6443 / #6445 stack

## Affected sites
- PR #6445 body — "Merge #6443 first, ... until it lands this branch carries its commit". Written when the base was `fix/memoized-issue-aliasing`; the base is now `main`.
- `89c487e7` on `perf-jit-issue-prefix` — the same fix as #6443, present in this PR's diff because `main` lacks it.

## Required outcome
- One of the two PRs is the one that lands the memoizer fix, and the other is rebased or closed. Either order is correct; the diff is only wrong if both land unrebased and #6443 is merged as an empty change.

## Open questions for the human
- Is #6443 still going in on its own, or is it folded into this one now that the base is `main`? If it's folded in, the body paragraph about merge order should go, and #6443 should be closed pointing here.

Pullfrog  | View workflow run | Using Claude Opus𝕏

zirkelc and others added 2 commits August 20, 2026 07:28
The three call sites emit identical code, so the template goes in a builder.
262 minified bytes off a classic z.object bundle, no change to what is emitted.
@colinhacks
colinhacks force-pushed the perf-jit-issue-prefix branch from 00e3b7b to ddfa5d4 Compare August 20, 2026 14:31
@pullfrog

pullfrog Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Your Claude subscription has hit its usage limit. It resets at 11pm (UTC). Re-trigger Pullfrog after the reset, or add an ANTHROPIC_API_KEY repo secret — Pullfrog routes around an exhausted subscription automatically when one is present.

Add repo secret → · Model settings → · Setup docs → · Ask in Discord →

Pullfrog  | Rerun failed job ➔View workflow run | via Pullfrog | Using Claude Opus𝕏

@colinhacks
colinhacks merged commit 937b5d0 into main Aug 20, 2026
7 of 8 checks passed
@colinhacks
colinhacks deleted the perf-jit-issue-prefix branch August 20, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants