Skip to content

Commit 13ccd02

Browse files
committed
ref(perf): keep executeField inlinable by extracting traced path
Hoist the resolve tracing decision once per `executeFields` / `executeFieldsSerially` call instead of re-checking `shouldTrace(resolveChannel)` per field, and move the traced branch's `traceMixed` call + ctx closure into a module-scope helper `invokeResolverWithTracing`. The latter is the load-bearing change: once `executeField`'s body referenced `traceMixed` and the `() => resolveFn(...)` closure, V8 stopped inlining `executeField` into `executeFields`, paying a real call frame per field. Extracting the two-function tail keeps `executeField`'s bytecode small enough to inline again. Effect on the introspection benchmark (the worst-case fan-out shape, ~1000 resolver calls per iteration, no subscribers attached): - with the inline ternary: -5.94% vs upstream - with this refactor: -2.05% / -2.30% across two paired runs Other benchmarks (list-sync, list-async, list-asyncIterable, object-async) sit at the noise floor with 95% CIs that cross zero. Adds a serial-mutation case to resolve-diagnostics-test to cover the `executeFieldsSerially` branch the hoisted snapshot now lives in.
1 parent bbddc0c commit 13ccd02

2 files changed

Lines changed: 69 additions & 5 deletions

File tree

src/execution/Executor.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
} from '../type/definition.js';
4545
import type { GraphQLSchema } from '../type/schema.js';
4646

47+
import type { MinimalTracingChannel } from '../diagnostics.js';
4748
import { resolveChannel, shouldTrace, traceMixed } from '../diagnostics.js';
4849

4950
import { AbortedGraphQLExecutionError } from './AbortedGraphQLExecutionError.js';
@@ -480,6 +481,9 @@ export class Executor<
480481
groupedFieldSet: GroupedFieldSet,
481482
positionContext: TPositionContext | undefined,
482483
): PromiseOrValue<ObjMap<unknown>> {
484+
const tracingChannel = shouldTrace(resolveChannel)
485+
? resolveChannel
486+
: undefined;
483487
return promiseReduce(
484488
groupedFieldSet,
485489
(results, [responseName, fieldDetailsList]) => {
@@ -493,6 +497,7 @@ export class Executor<
493497
fieldDetailsList,
494498
fieldPath,
495499
positionContext,
500+
tracingChannel,
496501
);
497502
if (result === undefined) {
498503
return results;
@@ -523,6 +528,9 @@ export class Executor<
523528
): PromiseOrValue<ObjMap<unknown>> {
524529
const results = Object.create(null);
525530
let containsPromise = false;
531+
const tracingChannel = shouldTrace(resolveChannel)
532+
? resolveChannel
533+
: undefined;
526534

527535
try {
528536
for (const [responseName, fieldDetailsList] of groupedFieldSet) {
@@ -533,6 +541,7 @@ export class Executor<
533541
fieldDetailsList,
534542
fieldPath,
535543
positionContext,
544+
tracingChannel,
536545
);
537546

538547
if (result !== undefined) {
@@ -574,6 +583,7 @@ export class Executor<
574583
fieldDetailsList: FieldDetailsList,
575584
path: Path,
576585
positionContext: TPositionContext | undefined,
586+
tracingChannel: MinimalTracingChannel | undefined,
577587
): PromiseOrValue<unknown> {
578588
const validatedExecutionArgs = this.validatedExecutionArgs;
579589
const { schema, contextValue, variableValues, hideSuggestions } =
@@ -615,11 +625,15 @@ export class Executor<
615625
// The resolve function's optional third argument is a context value that
616626
// is provided to every resolve function within an execution. It is commonly
617627
// used to represent an authenticated user, or request-specific caches.
618-
const result = shouldTrace(resolveChannel)
619-
? traceMixed(
620-
resolveChannel,
621-
buildResolveCtx(info, args, fieldDef.resolve === undefined),
622-
() => resolveFn(source, args, contextValue, info),
628+
const result = tracingChannel
629+
? invokeResolverWithTracing(
630+
tracingChannel,
631+
resolveFn,
632+
source,
633+
args,
634+
contextValue,
635+
info,
636+
fieldDef.resolve === undefined,
623637
)
624638
: resolveFn(source, args, contextValue, info);
625639

@@ -1430,3 +1444,24 @@ function buildResolveCtx(
14301444
},
14311445
};
14321446
}
1447+
1448+
/**
1449+
* Traced path for a single resolver call. Extracted as a module-scope function to increase likelihood of inlining.
1450+
*
1451+
* @internal
1452+
*/
1453+
function invokeResolverWithTracing(
1454+
tracingChannel: MinimalTracingChannel,
1455+
resolveFn: GraphQLFieldResolver<unknown, unknown>,
1456+
source: unknown,
1457+
args: { readonly [argument: string]: unknown },
1458+
contextValue: unknown,
1459+
info: GraphQLResolveInfo,
1460+
isTrivialResolver: boolean,
1461+
): PromiseOrValue<unknown> {
1462+
return traceMixed(
1463+
tracingChannel,
1464+
buildResolveCtx(info, args, isTrivialResolver),
1465+
() => resolveFn(source, args, contextValue, info),
1466+
);
1467+
}

src/execution/__tests__/resolve-diagnostics-test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,35 @@ describe('resolve diagnostics channel', () => {
178178
expect(endsSync.length).to.equal(4);
179179
});
180180

181+
it('emits per-field for serial mutation execution', async () => {
182+
const mutationSchema = new GraphQLSchema({
183+
query: new GraphQLObjectType({
184+
name: 'Query',
185+
fields: { dummy: { type: GraphQLString } },
186+
}),
187+
mutation: new GraphQLObjectType({
188+
name: 'Mutation',
189+
fields: {
190+
first: { type: GraphQLString, resolve: () => 'one' },
191+
second: { type: GraphQLString, resolve: () => 'two' },
192+
},
193+
}),
194+
});
195+
196+
active = collectEvents(resolveChannel);
197+
198+
await execute({
199+
schema: mutationSchema,
200+
document: parse('mutation M { first second }'),
201+
});
202+
203+
const starts = active.events.filter((e) => e.kind === 'start');
204+
expect(starts.map((e) => e.ctx.fieldName)).to.deep.equal([
205+
'first',
206+
'second',
207+
]);
208+
});
209+
181210
it('does nothing when no subscribers are attached', () => {
182211
const result = execute({
183212
schema,

0 commit comments

Comments
 (0)