Cache mapper instantiations #61505
Merged
Cache mapper instantiations #61505
Conversation
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
typescript-bot added the For Uncommitted Bug 
| This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Andarist commented Mar 30, 2025
Andarist commented Mar 30, 2025
| @typescript-bot test it |
| @typescript-bot test it |
| Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
| @jakebailey Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details |
| @jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown Hosts
Scenarios
Developer Information: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @typescript-bot pack this |
| Hey @jakebailey, something went wrong when looking for the build artifact. (You can check the log here). |
| @jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
| This is a set of instantiation benchmarks with results from These can be run against the installed TS version by executing this with import { bench } from "@ark/attest" type merge<base, props> = Omit<base, keyof props & keyof base> & props declare const merge: <l, r>(l: l, r: r) => merge<l, r> bench("functional(10)", () => { const a = merge({ a: 1 }, { b: 2 }) const b = merge(a, { c: 3 }) const c = merge(b, { d: 4 }) const d = merge(c, { e: 5 }) const e = merge(d, { f: 6 }) const f = merge(e, { g: 7 }) const g = merge(f, { h: 8 }) const h = merge(g, { i: 9 }) const i = merge(h, { j: 10 }) }).types([59386, "instantiations"]) bench("functional(11)", () => { const a = merge({ a: 1 }, { b: 2 }) const b = merge(a, { c: 3 }) const c = merge(b, { d: 4 }) const d = merge(c, { e: 5 }) const e = merge(d, { f: 6 }) const f = merge(e, { g: 7 }) const g = merge(f, { h: 8 }) const h = merge(g, { i: 9 }) const i = merge(h, { j: 10 }) const j = merge(i, { k: 11 }) }).types([177537, "instantiations"]) bench("functional(12)", () => { const a = merge({ a: 1 }, { b: 2 }) const b = merge(a, { c: 3 }) const c = merge(b, { d: 4 }) const d = merge(c, { e: 5 }) const e = merge(d, { f: 6 }) const f = merge(e, { g: 7 }) const g = merge(f, { h: 8 }) const h = merge(g, { i: 9 }) const i = merge(h, { j: 10 }) const j = merge(i, { k: 11 }) const k = merge(j, { l: 12 }) // should be linear relative to functional(10) and functional(11) }).types([531887, "instantiations"]) bench("functional(12) with spread", () => { const a = merge({ a: 1 }, { b: 2 }) const b = merge(a, { c: 3 }) const c = merge(b, { d: 4 }) const d = merge(c, { e: 5 }) const e = merge(d, { f: 6 }) const f = merge(e, { g: 7 }) const g = merge(f, { h: 8 }) const h = merge(g, { i: 9 }) const i = merge(h, { j: 10 }) const j = merge(i, { k: 11 }) const k = merge(j, { l: 12 }) return { ...i, ...j, ...k } // should be similar to functional(12) }).types([797849, "instantiations"]) type Type<t> = { merge: <r>(r: r) => Type<merge<t, r>> unwrapped: t } declare const a: Type<{ a: 1 }> bench("chained(10)", () => { const b = a.merge({ b: 2 }) const c = b.merge({ c: 3 }) const d = c.merge({ d: 4 }) const e = d.merge({ e: 5 }) const f = e.merge({ f: 6 }) const g = f.merge({ g: 7 }) const h = g.merge({ h: 8 }) const i = h.merge({ i: 9 }) const j = i.merge({ j: 10 }) }).types([178394, "instantiations"]) bench("chained(11)", () => { const b = a.merge({ b: 2 }) const c = b.merge({ c: 3 }) const d = c.merge({ d: 4 }) const e = d.merge({ e: 5 }) const f = e.merge({ f: 6 }) const g = f.merge({ g: 7 }) const h = g.merge({ h: 8 }) const i = h.merge({ i: 9 }) const j = i.merge({ j: 10 }) const k = j.merge({ k: 11 }) }).types([532898, "instantiations"]) bench("chained(12)", () => { const b = a.merge({ b: 2 }) const c = b.merge({ c: 3 }) const d = c.merge({ d: 4 }) const e = d.merge({ e: 5 }) const f = e.merge({ f: 6 }) const g = f.merge({ g: 7 }) const h = g.merge({ h: 8 }) const i = h.merge({ i: 9 }) const j = i.merge({ j: 10 }) const k = j.merge({ k: 11 }) const l = k.merge({ l: 12 }) // should be linear relative to chained(10) and chained(11) }).types([1596004, "instantiations"]) bench("chained(12) with spread", () => { const b = a.merge({ b: 2 }) const c = b.merge({ c: 3 }) const d = c.merge({ d: 4 }) const e = d.merge({ e: 5 }) const f = e.merge({ f: 6 }) const g = f.merge({ g: 7 }) const h = g.merge({ h: 8 }) const i = h.merge({ i: 9 }) const j = i.merge({ j: 10 }) const k = j.merge({ k: 11 }) const l = k.merge({ l: 12 }) return { ...i.unwrapped, ...j.unwrapped, ...k.unwrapped } // should be similar to chained(12) }).types([1684778, "instantiations"])I would expect the instantiation scaling to not be purely linear for this sort of operation as the number of props is increasing, but it should be relatively close. None of these scenarios should result in the exponential scaling we see currently. I'm also including the original end-to-end type benchmarks that identified this issue here. Once the minimal repo scaling has been addressed, we should sanity check that the scaling here is also fixed: import { bench } from '@ark/attest'; import { initTRPC } from '@trpc/server'; import { type } from 'arktype'; import { z } from 'zod'; const t = initTRPC.create(); // avoid pollution from one-time library setup bench.baseline(() => { const router = t.router({ baseline: t.procedure .input( z.object({ baseline: z.string(), }), ) .query(({ input }) => `hello ${input.baseline}`), arkBaseline: t.procedure .input( type({ baseline: 'string', }), ) .query(({ input }) => `hello ${input.baseline}`), }); }); const base = z.object({ a: z.string(), b: z.string(), c: z.string(), }); bench('non-sequential Zod type', async () => { const nonSequentialRouter = t.router({ query1: t.procedure.input(base).query(({ input }) => `hello ${input.a}`), mutation1: t.procedure .input(base) .mutation(({ input }) => `hello ${input.a}`), }); // this is relatively cheap }).types([1659, 'instantiations']); // even though sequential is totally equivalent to nonSequential, its // Zod representation is not reduced and still includes intermediate operations const sequential = base .partial() .merge(base) .pick({ a: true, b: true, c: true }) .omit({}) .merge(base); const base2 = z.object({ d: z.string(), e: z.string(), f: z.string(), }); bench('isolated sequential zod', () => { const sequential = base2 .partial() .merge(base2) .pick({ d: true, e: true, f: true }) .omit({}) .merge(base2); // this is expensive }).types([11420, 'instantiations']); bench('sequential Zod type', async () => { const sequentialRouter = t.router({ query1: t.procedure .input(sequential) .query(({ input }) => `hello ${input.a}`), mutation1: t.procedure .input(sequential) .mutation(({ input }) => `hello ${input.a}`), }); // but it's in combination with trpc that these sequentially evaluated // Zod types get out of control. instead of incurring a 1-time evaluation // cost, it seems it can't be cached and the extra inference cost // is incurred multiple times (even worse with deepPartial) }).types([49173, 'instantiations']); const arkBase = type({ a: 'string', b: 'string', c: 'string', }); bench('non-sequential arktype', async () => { const sequentialRouter = t.router({ query1: t.procedure.input(arkBase).query(({ input }) => `hello ${input.a}`), mutation1: t.procedure .input(arkBase) .mutation(({ input }) => `hello ${input.a}`), }); // realtively cheap }).types([2961, 'instantiations']); const arkBase2 = type({ d: 'string', e: 'string', f: 'string', }); bench('isolated sequential arktype', () => { arkBase2.partial().merge(arkBase2).pick('d', 'e', 'f').omit().merge(arkBase2); // these kind of operations are much cheaper in ArkType than Zod }).types([2336, 'instantiations']); const arkSequential = arkBase .partial() .merge(arkBase) .pick('a', 'b', 'c') .omit() .merge(arkBase); bench('sequential arktype', async () => { const sequentialRouter = t.router({ query1: t.procedure .input(arkSequential) .query(({ input }) => `hello ${input.a}`), mutation1: t.procedure .input(arkSequential) .mutation(({ input }) => `hello ${input.a}`), }); // even though hovering arkSequential is identical to hovering arkBase, // TS still seems to do a lot of repeated work inferring it somehow (though less than Zod) }).types([17906, 'instantiations']); |
Andarist force-pushed the cache-mapper-instaniations branch from 688f5d5 to b0f5c70 Compare 
| @ssalbdivad tested this in a trpc-based repo. TS 5.8: This branch (commit b0f5c70): |
| @typescript-bot test it |
| I took some of the numbers reported earlier to mean that the additional caching only decreases instantiations by half, but I see how it is much more dramatic in the micro benchmarks. So, I agree there's some merit to the additional caching, but ideally I'd like us to target it at the expensive instantiations instead of just caching all instantiations (which adds unnecessary overhead). I experimented with only caching instantiations of union and intersection types and that seems to do the trick with lower cost. So I'd suggest changing the PR to do that. I'm still slightly concerned that caches might not always be invalidated. For example, if the |
| @ahejlsberg This What concerns me is that whether or not individual instantiations are expensive, if there are O(3^N) of them, they will eventually dominate the check time of the entire type. IMO given @Andarist's second PR has no clear downsides in terms of check time or memory consumption, it seems worthwhile to just eliminate the logic that could lead to this scaling altogether. |
This is a valid concern 👍 I'll see what I can do to address it |
You might experiment with clearing all caches whenever a new inference is made. If that doesn't adversely affect performance I think that might be the best option. Also, as I mentioned earlier, you can remove some overhead by only caching results of union and intersection instantiations. |
1 task
| @typescript-bot test it |
| Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
| @jakebailey Here are the results of running the user tests with tsc comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
| Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
| @jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown Hosts
Scenarios
Developer Information: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
I've followed this advice and the numbers are still pretty much the same as the ones I mentioned here. Instantiations went up a little bit but by such a negligible number (+341), it doesn't matter at all.
The current version doesn't come with any noticeable overhead, from what I can tell. It seems to me doing this just for union/intersections would only complicate the code right now. If you feel strongly about it though, I could certainly run an extra experiment like this. |
jakebailey approved these changes Jun 5, 2025
There was a problem hiding this comment.
So, I feel like this must be good to go at this point. The code looks pretty simple, the benchmarks are good, and it should be straightforward to port into the Go codebase (eventually) as there aren't any goofy uses of maps / prototype modification anymore.
Are there any remaining concerns here?
This was referenced Jun 30, 2025
This was referenced Jul 3, 2025
| This was a game-changer PR for us. Thank you!
|
| What repo is that? Is it public? |
| It's a private repo unfortunately. It is for the Klarna app (react-native, web, etc). But we'd be happy to test out anything internally and get back to you if want 😄 |
| I want a public repo for our benchmarks, unfortunately |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
9 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.
@ssalbdivad found out that a chain of somewhat trivial operations can easily end up with dreaded "Type instantiation is excessively deep and possibly infinite".
Two different reproductions were created:
We were able to bisect both to two different past PRs - one of which is quite old:
⚠️ that said - the "chain setup" avoids the error better but the property type lookup can still lead to the error. So, in some sense, the error just shifted place.I dug into this and what I found out is, those types refer to the same "base" types - they are chained from them and
instantiateTyperepeats the work on them a lot. At least part of the problem is thatgetObjectTypeInstantiationcallsinstantiateTypes(type.aliasTypeArguments, mapper)to create part of its cache key. So it can't even check if the result is cached before it instantiates those. This problem compounds heavily in longer chains like this.So I toyed with possible solutions and I realized that caching results on mappers could help here... to some extent at least. I don't think it's the best solution but I also don't yet understand this problem to its core to propose anything better.
It looks like this has very positive impact on instantiation counts across the test suite. It surely trades some memory for it though. It would be interesting to see perf results for this and the extended test suite run (even if only to get new data points for further investigation).
EDIT:// @ssalbdivad has tested this in some trpc-based repositories and the check time got halved for them. The current version is also able to typecheck the 50-item deep "chains" as it can be seen in the added
long*tests. Those previously were only instantiable to a certain depth (smth between 15-25, depending on the test case) and the last items in the chain were being instantiated really slow. Now those longer ones are handled pretty fast.