perf(graphql): trim per-resolver allocations#8309
Merged
Merged
Conversation
Contributor
Overall package sizeSelf size: 5.78 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 6dad8df | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-05-08 11:28:21 Comparing candidate commit 6dad8df in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1750 metrics, 94 unstable metrics. |
BridgeAR
force-pushed
the
BridgeAR/2026-05-07-perf-graphql-resolver
branch
from
May 7, 2026 17:57
6c8ac37 to
b015de0
Compare
`resolve.start` and `assertField` walk the same resolver data several times per call. The following improvements are implemented: 1. `info.fieldNodes.find(...)` collapses to `info.fieldNodes[0]`; the executor only hands FieldNode siblings to the resolver. 2. `pathToArray` switches to a count-then-fill walk. 3. `getResolverInfo` allocates `resolverVars` lazily. 4. `shouldInstrument` short-circuits when `config.depth < 0` (the default). 5. `getPath` / `withCollapse` fold into a single walk. 6. `wrapExecute` reuses one `documentSources.get(document)` lookup. 7. `addVariableTags` returns early when there are no variables and no filter. Drive-by fix: * `finishResolvers` drops the `.reverse()` pass; `field.finishTime` already pins each span's wall-clock finish. * `wrapFields` uses `Object.values(type._fields)` directly.
BridgeAR
force-pushed
the
BridgeAR/2026-05-07-perf-graphql-resolver
branch
from
May 8, 2026 09:29
b015de0 to
6dad8df
Compare
BridgeAR
marked this pull request as ready for review
May 8, 2026 10:00
bengl
approved these changes
May 11, 2026
bengl
approved these changes
May 11, 2026
Merged
rochdev
pushed a commit
that referenced
this pull request
May 13, 2026
`resolve.start` and `assertField` walk the same resolver data several times per call. The following improvements are implemented: 1. `info.fieldNodes.find(...)` collapses to `info.fieldNodes[0]`; the executor only hands FieldNode siblings to the resolver. 2. `pathToArray` switches to a count-then-fill walk. 3. `getResolverInfo` allocates `resolverVars` lazily. 4. `shouldInstrument` short-circuits when `config.depth < 0` (the default). 5. `getPath` / `withCollapse` fold into a single walk. 6. `wrapExecute` reuses one `documentSources.get(document)` lookup. 7. `addVariableTags` returns early when there are no variables and no filter. Drive-by fix: * `finishResolvers` drops the `.reverse()` pass; `field.finishTime` already pins each span's wall-clock finish. * `wrapFields` uses `Object.values(type._fields)` directly.
crysmags
added a commit
that referenced
this pull request
May 14, 2026
After merging master, #8309's perf wins target functions that either no longer exist (wrapExecute / wrapFields / pathToArray / finishResolvers moved out of instrumentations/graphql.js) or live in a deleted file (resolve.js was folded into execute.js). Port the still-applicable changes onto the migrated execute plugin: #1 _startResolveSpan uses info.fieldNodes[0] instead of .find(fn => fn.kind === 'Field'). The executor only hands FieldNode siblings. #2 pathToArray switches to count-then-fill (drops the trailing .reverse() and the push-grow). #3 getResolverInfo lazy-allocates resolverVars: skips both the up-front {} and the directive-loop assignment when args is empty and no directives have arguments. #4 shouldInstrument early-returns on config.depth < 0 (no-limit, the default-enabled case) before walking the path. #5 buildCollapsedPathString builds the collapsed key in one walk; drops the intermediate collapsedPath array. iastResolveCh.publish now passes raw path / pathString, matching the contract the old resolve.js had with the IAST subscriber. drive-by wrapFields iterates Object.values(type._fields) directly. #7 (addVariableTags early-return) already auto-merged during the master merge. #6 (single documentSources.get in wrapExecute) is already the shape of bindStart in the migrated plugin. The reverted depth=0 bindStart short-circuit (156a557 / ca2d041) sat at a different position — inside bindStart, gated on _depthDisabled (config.depth === 0). It was rolled back because piling it on top of the resolveAsync depth-flag fix didn't move plugin-graphql-long-with- depth-off any further. This port stays inside shouldInstrument, fires on config.depth < 0 (the *enabled* default), and targets the hot path on plugin-graphql-long. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
crysmags
added a commit
that referenced
this pull request
May 19, 2026
After merging master, #8309's perf wins target functions that either no longer exist (wrapExecute / wrapFields / pathToArray / finishResolvers moved out of instrumentations/graphql.js) or live in a deleted file (resolve.js was folded into execute.js). Port the still-applicable changes onto the migrated execute plugin: #1 _startResolveSpan uses info.fieldNodes[0] instead of .find(fn => fn.kind === 'Field'). The executor only hands FieldNode siblings. #2 pathToArray switches to count-then-fill (drops the trailing .reverse() and the push-grow). #3 getResolverInfo lazy-allocates resolverVars: skips both the up-front {} and the directive-loop assignment when args is empty and no directives have arguments. #4 shouldInstrument early-returns on config.depth < 0 (no-limit, the default-enabled case) before walking the path. #5 buildCollapsedPathString builds the collapsed key in one walk; drops the intermediate collapsedPath array. iastResolveCh.publish now passes raw path / pathString, matching the contract the old resolve.js had with the IAST subscriber. drive-by wrapFields iterates Object.values(type._fields) directly. #7 (addVariableTags early-return) already auto-merged during the master merge. #6 (single documentSources.get in wrapExecute) is already the shape of bindStart in the migrated plugin. The reverted depth=0 bindStart short-circuit (156a557 / ca2d041) sat at a different position — inside bindStart, gated on _depthDisabled (config.depth === 0). It was rolled back because piling it on top of the resolveAsync depth-flag fix didn't move plugin-graphql-long-with- depth-off any further. This port stays inside shouldInstrument, fires on config.depth < 0 (the *enabled* default), and targets the hot path on plugin-graphql-long. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
crysmags
added a commit
that referenced
this pull request
May 20, 2026
Replaces the shimmer-based graphql instrumentation in
packages/datadog-instrumentations/src/graphql.js with orchestrion AST
rewriting for the top-level execute / parse / validate entry points
(both CJS and ESM in graphql >= 0.10 and @graphql-tools/executor).
Plugin architecture changes:
- packages/datadog-plugin-graphql/src/execute.js: takes over resolver
wrapping that used to live in instrumentations; uses a single
rootCtx per execute() call to coordinate execute-span creation and
per-resolver bookkeeping. Resolve spans are recorded inline at
first-encounter and finished from the resolver callback so encoder
state stays hot across span finalization.
- packages/datadog-plugin-graphql/src/resolve.js: deleted. The
resolve-plugin sub-class was needed only because the old shimmer
architecture published per-field events through a separate
plugin; the orchestrion-based design folds that work into the
execute plugin and uses a module-level _depthDisabled flag to
short-circuit when tracer.use('graphql', { depth: 0 }).
- packages/datadog-plugin-graphql/src/parse.js,validate.js: hooked
via orchestrion publishes; documentSources mapping moved onto the
parse plugin so the execute plugin can read raw query source for
graphql.source tags + IAST taint tracking.
Correctness fixes wired up alongside the migration:
- IAST taint tracking: orchestrion's bindStart now fires the same
apm:graphql:resolve:start payload the old shimmer wrapper produced,
using the actual args object graphql constructs internally rather
than an AST-rebuilt copy. Restores taint propagation for hardcoded
literal resolver arguments (e.g. books(title: "ls")) which the
initial orchestrion port broke.
- AppSec resolver blocking: passes the abort controller directly to
the channel payload so AppSec subscribers can abort the resolver
synchronously. packages/dd-trace/src/appsec/graphql.js updated to
read req via getActiveRequest() (the storage('legacy') call only
worked under the old shimmer setup).
- Channel payload shape change in packages/dd-trace/src/appsec/
channels.js to match the new bindStart contract.
Performance work folded in (per-execute and per-resolver):
- Lazy-build info object and resolver args; only construct when the
plugin actually needs them.
- Eliminate per-field channel publishing — the per-execute rootCtx
holds field records and we publish nothing per resolver in the
APM-only path.
- Collapse redundant path walks in the resolve bindStart so
pathToArray runs once per resolver call.
- Consolidate per-field stash + finish handler into one shared
object; avoids reallocating ctx objects per resolver.
- Short-circuit re-entrant execute() on the same contextValue
(covers yoga's normalizedExecutor pattern that would otherwise
double-span).
- Hot fast-path when depth=0: cache the disabled state on a
module-level flag in configure() so resolveAsync bails on a single
property read, matching master's startResolveCh.hasSubscribers
cost.
- Port of #8309's signature-pipeline optimization onto the migrated
execute plugin.
All 231 plugin-graphql tests pass.
Local sirun on plugin-graphql-long (100q × 5 iters per Node version)
shows per-process user-CPU vs master:
with-depth-on-max: -340 ms (-31%)
with-depth-and-collapse-off: tied
with-tracer-graphql-disabled: -50 ms (-7%)
with-depth-off: +130 ms (+21%)
The depth=0 regression is the only configuration where PR is slower
than master. It traces to orchestrion's publish format carrying raw
function args (master's shimmer wrapper pre-computed
{operation, args, docSource} before publishing). For variants with any
per-resolver work, PR's structural wins overtake this cost many times
over.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
crysmags
added a commit
that referenced
this pull request
Jun 3, 2026
Replaces the shimmer-based graphql instrumentation in
packages/datadog-instrumentations/src/graphql.js with orchestrion AST
rewriting for the top-level execute / parse / validate entry points
(both CJS and ESM in graphql >= 0.10 and @graphql-tools/executor).
Plugin architecture changes:
- packages/datadog-plugin-graphql/src/execute.js: takes over resolver
wrapping that used to live in instrumentations; uses a single
rootCtx per execute() call to coordinate execute-span creation and
per-resolver bookkeeping. Resolve spans are recorded inline at
first-encounter and finished from the resolver callback so encoder
state stays hot across span finalization.
- packages/datadog-plugin-graphql/src/resolve.js: deleted. The
resolve-plugin sub-class was needed only because the old shimmer
architecture published per-field events through a separate
plugin; the orchestrion-based design folds that work into the
execute plugin and uses a module-level _depthDisabled flag to
short-circuit when tracer.use('graphql', { depth: 0 }).
- packages/datadog-plugin-graphql/src/parse.js,validate.js: hooked
via orchestrion publishes; documentSources mapping moved onto the
parse plugin so the execute plugin can read raw query source for
graphql.source tags + IAST taint tracking.
Correctness fixes wired up alongside the migration:
- IAST taint tracking: orchestrion's bindStart now fires the same
apm:graphql:resolve:start payload the old shimmer wrapper produced,
using the actual args object graphql constructs internally rather
than an AST-rebuilt copy. Restores taint propagation for hardcoded
literal resolver arguments (e.g. books(title: "ls")) which the
initial orchestrion port broke.
- AppSec resolver blocking: passes the abort controller directly to
the channel payload so AppSec subscribers can abort the resolver
synchronously. packages/dd-trace/src/appsec/graphql.js updated to
read req via getActiveRequest() (the storage('legacy') call only
worked under the old shimmer setup).
- Channel payload shape change in packages/dd-trace/src/appsec/
channels.js to match the new bindStart contract.
Performance work folded in (per-execute and per-resolver):
- Lazy-build info object and resolver args; only construct when the
plugin actually needs them.
- Eliminate per-field channel publishing — the per-execute rootCtx
holds field records and we publish nothing per resolver in the
APM-only path.
- Collapse redundant path walks in the resolve bindStart so
pathToArray runs once per resolver call.
- Consolidate per-field stash + finish handler into one shared
object; avoids reallocating ctx objects per resolver.
- Short-circuit re-entrant execute() on the same contextValue
(covers yoga's normalizedExecutor pattern that would otherwise
double-span).
- Hot fast-path when depth=0: cache the disabled state on a
module-level flag in configure() so resolveAsync bails on a single
property read, matching master's startResolveCh.hasSubscribers
cost.
- Port of #8309's signature-pipeline optimization onto the migrated
execute plugin.
All 231 plugin-graphql tests pass.
Local sirun on plugin-graphql-long (100q × 5 iters per Node version)
shows per-process user-CPU vs master:
with-depth-on-max: -340 ms (-31%)
with-depth-and-collapse-off: tied
with-tracer-graphql-disabled: -50 ms (-7%)
with-depth-off: +130 ms (+21%)
The depth=0 regression is the only configuration where PR is slower
than master. It traces to orchestrion's publish format carrying raw
function args (master's shimmer wrapper pre-computed
{operation, args, docSource} before publishing). For variants with any
per-resolver work, PR's structural wins overtake this cost many times
over.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
crysmags
added a commit
that referenced
this pull request
Jun 3, 2026
Replaces the shimmer-based graphql instrumentation in
packages/datadog-instrumentations/src/graphql.js with orchestrion AST
rewriting for the top-level execute / parse / validate entry points
(both CJS and ESM in graphql >= 0.10 and @graphql-tools/executor).
Plugin architecture changes:
- packages/datadog-plugin-graphql/src/execute.js: takes over resolver
wrapping that used to live in instrumentations; uses a single
rootCtx per execute() call to coordinate execute-span creation and
per-resolver bookkeeping. Resolve spans are recorded inline at
first-encounter and finished from the resolver callback so encoder
state stays hot across span finalization.
- packages/datadog-plugin-graphql/src/resolve.js: deleted. The
resolve-plugin sub-class was needed only because the old shimmer
architecture published per-field events through a separate
plugin; the orchestrion-based design folds that work into the
execute plugin and uses a module-level _depthDisabled flag to
short-circuit when tracer.use('graphql', { depth: 0 }).
- packages/datadog-plugin-graphql/src/parse.js,validate.js: hooked
via orchestrion publishes; documentSources mapping moved onto the
parse plugin so the execute plugin can read raw query source for
graphql.source tags + IAST taint tracking.
Correctness fixes wired up alongside the migration:
- IAST taint tracking: orchestrion's bindStart now fires the same
apm:graphql:resolve:start payload the old shimmer wrapper produced,
using the actual args object graphql constructs internally rather
than an AST-rebuilt copy. Restores taint propagation for hardcoded
literal resolver arguments (e.g. books(title: "ls")) which the
initial orchestrion port broke.
- AppSec resolver blocking: passes the abort controller directly to
the channel payload so AppSec subscribers can abort the resolver
synchronously. packages/dd-trace/src/appsec/graphql.js updated to
read req via getActiveRequest() (the storage('legacy') call only
worked under the old shimmer setup).
- Channel payload shape change in packages/dd-trace/src/appsec/
channels.js to match the new bindStart contract.
Performance work folded in (per-execute and per-resolver):
- Lazy-build info object and resolver args; only construct when the
plugin actually needs them.
- Eliminate per-field channel publishing — the per-execute rootCtx
holds field records and we publish nothing per resolver in the
APM-only path.
- Collapse redundant path walks in the resolve bindStart so
pathToArray runs once per resolver call.
- Consolidate per-field stash + finish handler into one shared
object; avoids reallocating ctx objects per resolver.
- Short-circuit re-entrant execute() on the same contextValue
(covers yoga's normalizedExecutor pattern that would otherwise
double-span).
- Hot fast-path when depth=0: cache the disabled state on a
module-level flag in configure() so resolveAsync bails on a single
property read, matching master's startResolveCh.hasSubscribers
cost.
- Port of #8309's signature-pipeline optimization onto the migrated
execute plugin.
All 231 plugin-graphql tests pass.
Local sirun on plugin-graphql-long (100q × 5 iters per Node version)
shows per-process user-CPU vs master:
with-depth-on-max: -340 ms (-31%)
with-depth-and-collapse-off: tied
with-tracer-graphql-disabled: -50 ms (-7%)
with-depth-off: +130 ms (+21%)
The depth=0 regression is the only configuration where PR is slower
than master. It traces to orchestrion's publish format carrying raw
function args (master's shimmer wrapper pre-computed
{operation, args, docSource} before publishing). For variants with any
per-resolver work, PR's structural wins overtake this cost many times
over.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
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.
Summary
Seven small wins on
resolve.start/assertField:info.fieldNodes.find(...)collapses toinfo.fieldNodes[0]; theexecutor only hands FieldNode siblings to the resolver.
pathToArrayswitches to a count-then-fill walk.getResolverInfoallocatesresolverVarslazily.shouldInstrumentshort-circuits on the defaultconfig.depth < 0.getPath/withCollapsefold into a single walk.wrapExecutereuses onedocumentSources.get(document)lookup.addVariableTagsreturns early when there are no variables and nofilter.
Bench:
pathToArraydepth-2 1.95x;getResolverInfo5-arg 2.36x.Drive-by fix:
finishResolversdrops the.reverse()pass;field.finishTimealready pins each span's wall-clock finish.
wrapFieldsusesObject.values(type._fields)directly.