perf: reduce context copying in resolver pipeline via Object.create#1085
perf: reduce context copying in resolver pipeline via Object.create#1085hugo-ccabral wants to merge 1 commit intomainfrom
Conversation
withResolveChain, withResolveChainOfType, and invokeResolverWithProps were spreading the entire context (10+ fields) just to change 1-2 fields. Use Object.create for prototypal inheritance so only changed properties are allocated. These functions are called recursively in the resolve pipeline, so this saves O(depth * fields) allocations per resolve chain. Co-authored-by: Cursor <cursoragent@cursor.com>
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughIntroduces two new helper functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
engine/core/resolver.ts (1)
379-383: Avoid double context derivation on theresolveIdpath.At Line [381] + Line [387], this branch creates one derived context for
resolveId, then another inwithResolveChain. You can allocate once and setresolveIdon that single derived object.♻️ Suggested simplification
- let chainCtx = ctx; - if (resolveId) { - chainCtx = Object.create(ctx) as typeof ctx; - chainCtx.resolveId = resolveId; - } + const chainCtx = withResolveChain(ctx, { type: "prop", value: _key }); + if (resolveId) { + chainCtx.resolveId = resolveId; + } const resolved = await resolvePropsWithHints( props[key], hint as HintNode<T[typeof key]>, - withResolveChain(chainCtx, { type: "prop", value: _key }), + chainCtx, opts, );Also applies to: 387-387
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@engine/core/resolver.ts` around lines 379 - 383, The code currently creates two derived contexts when resolveId is set (once by creating chainCtx from ctx and then again inside withResolveChain); change this to allocate a single derived context: if resolveId is present, create one Object.create(ctx) instance, set chainCtx.resolveId = resolveId, and pass that same chainCtx into withResolveChain so no second derivation occurs; update usages of chainCtx, ctx, resolveId, and the withResolveChain call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@engine/core/resolver.ts`:
- Around line 264-266: The created contexts use Object.create(ctx) (e.g., where
derived.resolveChain is set) which makes resolvables and resolvers inherited and
thus lost when middleware spreads ctx with { ...ctx, next }; change those
Object.create(ctx) usages (the derived context creations around resolveChain and
similar spots) to create a shallow own-property clone instead (e.g., const
derived = { ...ctx } as TContext) and then set derived.resolveChain =
ctx.resolveChain.concat(resolverType); repeat this change for the other two
occurrences so resolvables and resolvers become own properties and survive the
middleware spread.
---
Nitpick comments:
In `@engine/core/resolver.ts`:
- Around line 379-383: The code currently creates two derived contexts when
resolveId is set (once by creating chainCtx from ctx and then again inside
withResolveChain); change this to allocate a single derived context: if
resolveId is present, create one Object.create(ctx) instance, set
chainCtx.resolveId = resolveId, and pass that same chainCtx into
withResolveChain so no second derivation occurs; update usages of chainCtx, ctx,
resolveId, and the withResolveChain call accordingly.
| const derived = Object.create(ctx) as TContext; | ||
| derived.resolveChain = ctx.resolveChain.concat(resolverType); | ||
| return derived; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n engine/core/resolver.ts | head -300 | tail -50Repository: deco-cx/deco
Length of output: 1750
🏁 Script executed:
cat -n engine/core/resolver.ts | sed -n '250,290p'Repository: deco-cx/deco
Length of output: 1425
🏁 Script executed:
cat -n engine/core/resolver.ts | sed -n '450,470p'Repository: deco-cx/deco
Length of output: 766
🏁 Script executed:
# Search for patterns that enumerate own keys on ctx/context objects
rg -nP --type=ts -C2 \
'Object\.(keys|values|entries)\(\s*(ctx|context)\s*\)|for\s*\(\s*const\s+\w+\s+in\s+(ctx|context)\s*\)|\{\s*\.\.\.\s*(ctx|context)\s*\}|JSON\.stringify\(\s*(ctx|context)\s*\)' engine/Repository: deco-cx/deco
Length of output: 38
🏁 Script executed:
# Search for resolver function implementations
rg -nP --type=ts 'export\s+(const|function)\s+\w*[Rr]esolver|const\s+\w*[Rr]esolver\s*=' engine/core/ | head -20Repository: deco-cx/deco
Length of output: 539
🏁 Script executed:
# Search for spread operator on context-like variables more broadly
rg -nP --type=ts '\{\s*\.\.\.' engine/ | grep -i 'ctx\|context' | head -20Repository: deco-cx/deco
Length of output: 612
🏁 Script executed:
# Search for Object.keys, Object.values, Object.entries usage
rg -nP --type=ts 'Object\.(keys|values|entries)' engine/ | head -20Repository: deco-cx/deco
Length of output: 1853
🏁 Script executed:
# Look for JSON.stringify usage on context
rg -nP --type=ts 'JSON\.stringify' engine/ | grep -i 'ctx\|context'Repository: deco-cx/deco
Length of output: 167
🏁 Script executed:
# Check if there are any actual breaking usages by searching all resolver implementations
rg -nP --type=ts 'for\s*\(\s*\w+\s+in\s+' engine/core/resolver.tsRepository: deco-cx/deco
Length of output: 38
🏁 Script executed:
cat -n engine/middleware.ts | sed -n '25,45p'Repository: deco-cx/deco
Length of output: 551
🏁 Script executed:
# Get more context around the middleware resolver call
cat -n engine/middleware.ts | sed -n '1,60p'Repository: deco-cx/deco
Length of output: 1374
🏁 Script executed:
# Check what calls middleware and if it uses the Object.create'd contexts
rg -nP --type=ts 'withResolveChain|withResolveChainOfType' engine/Repository: deco-cx/deco
Length of output: 492
🏁 Script executed:
# Check if the context spreading in tests would actually catch this issue
cat -n engine/core/mod.test.ts | sed -n '100,120p'Repository: deco-cx/deco
Length of output: 779
🏁 Script executed:
# Check what properties are set as own vs inherited in context creation
rg -nP --type=ts 'resolveChain|monitoring|resolverId|resolvers|resolvables' engine/core/resolver.ts | head -30Repository: deco-cx/deco
Length of output: 1354
🏁 Script executed:
# Search for middleware implementations in the codebase
fd -e ts -e tsx | xargs grep -l 'ResolverMiddleware' | head -10Repository: deco-cx/deco
Length of output: 159
🏁 Script executed:
# Check if any middleware actually accesses context properties
rg -nP --type=ts 'ctx\.(resolvers|resolvables|monitoring|resolverId)' engine/ | head -20Repository: deco-cx/deco
Length of output: 683
🏁 Script executed:
# Trace the flow: where are Object.create'd contexts used in resolver chain
rg -nP --type=ts 'withResolveChain|compose' engine/core/resolver.ts | head -20Repository: deco-cx/deco
Length of output: 372
🏁 Script executed:
# Check if tests would catch this issue with middleware
rg -nP --type=ts 'compose.*middleware' engine/Repository: deco-cx/deco
Length of output: 38
🏁 Script executed:
# Check what happens to inherited properties when spread
cat -n engine/middleware.ts | sed -n '20,40p'Repository: deco-cx/deco
Length of output: 790
Compatibility break: middleware spread loses inherited context properties
Using Object.create(ctx) at lines 264, 275, and 459 makes resolvables and resolvers inherited rather than own properties. The middleware composer at line 34 spreads context with { ...ctx, next }, which copies only own properties, losing the inherited resolvables and resolvers fields required for resolver execution. Any middleware implementation or resolver flow through the middleware layer will break when these inherited properties are accessed.
Problematic code location
engine/middleware.ts:34
return resolver(p, { ...ctx, next });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@engine/core/resolver.ts` around lines 264 - 266, The created contexts use
Object.create(ctx) (e.g., where derived.resolveChain is set) which makes
resolvables and resolvers inherited and thus lost when middleware spreads ctx
with { ...ctx, next }; change those Object.create(ctx) usages (the derived
context creations around resolveChain and similar spots) to create a shallow
own-property clone instead (e.g., const derived = { ...ctx } as TContext) and
then set derived.resolveChain = ctx.resolveChain.concat(resolverType); repeat
this change for the other two occurrences so resolvables and resolvers become
own properties and survive the middleware spread.
1 participant
Summary
Object.create()inwithResolveChain(),withResolveChainOfType(), andinvokeResolverWithProps()Object.create(ctx)uses prototypal inheritance — only the changed property is allocated, all others are inherited through the prototype chainresolveIdoverride inresolvePropsWithHints()to avoid a spreadContext
The resolver pipeline calls these functions recursively for every prop with hints. A page with 30 sections, each with nested props, generates hundreds of context copies. CPU profile showed the full resolver pipeline at ~3% CPU, and memory profile showed context/closure allocations contributing to the 70% framework allocation share.
Safety
All consumers of these context objects access known property names (
ctx.resolveChain,ctx.monitoring, etc.) — prototypal inheritance works correctly for property access. No code iterates over context keys or usesObject.keys().Test plan
Summary by cubic
Reduced context copying in the resolver pipeline by using Object.create to derive lightweight contexts. This cuts allocations in deep resolve chains and should lower CPU and memory overhead.
Written for commit 4d7bc23. Summary will update on new commits.
Summary by CodeRabbit