Skip to content

Commit a001dd3

Browse files
committed
ref(perf): inline no-subscriber fast path at tracing emission sites
On parse/validate/execute/subscribe entry points, the previous `maybeTrace*` wrapper allocated a ctx-factory closure and a body closure on every call before short-circuiting on `hasSubscribers`. Inline the gate as `if (!channel?.hasSubscribers) return impl(args)` so closures and ctx objects are only allocated when a subscriber is attached. Recovers roughly half of the tracing-channel overhead seen in benchmarks. List-sync is back within noise of the pre-tracing baseline. Introspection remains ~3% slower due to the per-field resolve gate; the shape-change fix (dropping `_resolveChannel` from Executor) in the previous commit already captured the other half of that regression.
1 parent a2e72f3 commit a001dd3

5 files changed

Lines changed: 147 additions & 113 deletions

File tree

src/diagnostics.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,13 @@ export function traceMixed<T>(
187187
});
188188
});
189189
}
190+
191+
/**
192+
* Check if a channel is defined and has subscribers.
193+
*/
194+
export function shouldTrace(
195+
channel: MinimalTracingChannel | undefined,
196+
): channel is MinimalTracingChannel {
197+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare
198+
return channel !== undefined && channel.hasSubscribers !== false;
199+
}

src/execution/Executor.ts

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

47-
import type { MinimalTracingChannel } from '../diagnostics.js';
48-
import { getChannels, maybeTraceMixed, shouldTrace } from '../diagnostics.js';
47+
import { resolveChannel, shouldTrace, traceMixed } from '../diagnostics.js';
4948

5049
import { withCancellation } from './cancellablePromise.js';
5150
import type {
@@ -245,12 +244,6 @@ export class Executor<
245244
values: ReadonlyArray<PromiseOrValue<T>>,
246245
) => Promise<Array<T>>;
247246

248-
// Resolved once per Executor so the per-field gate in `executeField` is a
249-
// single member read + null check, not a `getChannels()?.resolve` walk +
250-
// `hasSubscribers` read on every resolution. Undefined when diagnostics
251-
// are off or nobody is listening at construction time.
252-
_resolveChannel: MinimalTracingChannel | undefined;
253-
254247
constructor(
255248
validatedExecutionArgs: ValidatedExecutionArgs,
256249
sharedExecutionContext?: SharedExecutionContext,
@@ -260,11 +253,6 @@ export class Executor<
260253
this.abortReason = defaultAbortReason;
261254
this.collectedErrors = new CollectedErrors();
262255

263-
const resolveChannel = getChannels()?.resolve;
264-
this._resolveChannel = shouldTrace(resolveChannel)
265-
? resolveChannel
266-
: undefined;
267-
268256
if (sharedExecutionContext === undefined) {
269257
this.resolverAbortController = new AbortController();
270258
this.sharedExecutionContext = createSharedExecutionContext(
@@ -622,10 +610,10 @@ export class Executor<
622610
// The resolve function's optional third argument is a context value that
623611
// is provided to every resolve function within an execution. It is commonly
624612
// used to represent an authenticated user, or request-specific caches.
625-
const result = this._resolveChannel
626-
? maybeTraceMixed(
627-
'resolve',
628-
() => buildResolveCtx(info, args, fieldDef.resolve === undefined),
613+
const result = shouldTrace(resolveChannel)
614+
? traceMixed(
615+
resolveChannel,
616+
buildResolveCtx(info, args, fieldDef.resolve === undefined),
629617
() => resolveFn(source, args, contextValue, info),
630618
)
631619
: resolveFn(source, args, contextValue, info);

src/execution/execute.ts

Lines changed: 121 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ import type { GraphQLSchema } from '../type/schema.js';
3434

3535
import { getOperationAST } from '../utilities/getOperationAST.js';
3636

37-
import { maybeTraceMixed } from '../diagnostics.js';
37+
import {
38+
executeChannel,
39+
subscribeChannel,
40+
traceMixed,
41+
} from '../diagnostics.js';
3842

3943
import { cancellablePromise } from './cancellablePromise.js';
4044
import type { FieldDetailsList, FragmentDetails } from './collectFields.js';
@@ -58,26 +62,24 @@ const UNEXPECTED_EXPERIMENTAL_DIRECTIVES =
5862
* resolution of the operation AST to a lazy getter so the cost of walking
5963
* the document is only paid if a subscriber reads it.
6064
*/
61-
function buildExecuteCtxFromArgs(args: ExecutionArgs): () => object {
62-
return () => {
63-
let operation: OperationDefinitionNode | null | undefined;
64-
const resolveOperation = (): OperationDefinitionNode | null | undefined => {
65-
if (operation === undefined) {
66-
operation = getOperationAST(args.document, args.operationName);
67-
}
68-
return operation;
69-
};
70-
return {
71-
document: args.document,
72-
schema: args.schema,
73-
variableValues: args.variableValues,
74-
get operationName() {
75-
return args.operationName ?? resolveOperation()?.name?.value;
76-
},
77-
get operationType() {
78-
return resolveOperation()?.operation;
79-
},
80-
};
65+
function buildExecuteCtxFromArgs(args: ExecutionArgs): object {
66+
let operation: OperationDefinitionNode | null | undefined;
67+
const resolveOperation = (): OperationDefinitionNode | null | undefined => {
68+
if (operation === undefined) {
69+
operation = getOperationAST(args.document, args.operationName);
70+
}
71+
return operation;
72+
};
73+
return {
74+
document: args.document,
75+
schema: args.schema,
76+
variableValues: args.variableValues,
77+
get operationName() {
78+
return args.operationName ?? resolveOperation()?.name?.value;
79+
},
80+
get operationType() {
81+
return resolveOperation()?.operation;
82+
},
8183
};
8284
}
8385

@@ -90,14 +92,14 @@ function buildExecuteCtxFromArgs(args: ExecutionArgs): () => object {
9092
*/
9193
function buildExecuteCtxFromValidatedArgs(
9294
args: ValidatedExecutionArgs,
93-
): () => object {
94-
return () => ({
95+
): object {
96+
return {
9597
operation: args.operation,
9698
schema: args.schema,
9799
variableValues: args.variableValues,
98100
operationName: args.operation.name?.value,
99101
operationType: args.operation.operation,
100-
});
102+
};
101103
}
102104

103105
/**
@@ -117,23 +119,27 @@ function buildExecuteCtxFromValidatedArgs(
117119
* delivery.
118120
*/
119121
export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
120-
return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => {
121-
if (
122-
args.schema.getDirective('defer') ||
123-
args.schema.getDirective('stream')
124-
) {
125-
throw new Error(UNEXPECTED_EXPERIMENTAL_DIRECTIVES);
126-
}
122+
if (!executeChannel?.hasSubscribers) {
123+
return executeImpl(args);
124+
}
125+
return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () =>
126+
executeImpl(args),
127+
);
128+
}
127129

128-
const validatedExecutionArgs = validateExecutionArgs(args);
130+
function executeImpl(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
131+
if (args.schema.getDirective('defer') || args.schema.getDirective('stream')) {
132+
throw new Error(UNEXPECTED_EXPERIMENTAL_DIRECTIVES);
133+
}
129134

130-
// Return early errors if execution context failed.
131-
if (!('schema' in validatedExecutionArgs)) {
132-
return { errors: validatedExecutionArgs };
133-
}
135+
const validatedExecutionArgs = validateExecutionArgs(args);
134136

135-
return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs);
136-
});
137+
// Return early errors if execution context failed.
138+
if (!('schema' in validatedExecutionArgs)) {
139+
return { errors: validatedExecutionArgs };
140+
}
141+
142+
return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs);
137143
}
138144

139145
/**
@@ -151,39 +157,57 @@ export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
151157
export function experimentalExecuteIncrementally(
152158
args: ExecutionArgs,
153159
): PromiseOrValue<ExecutionResult | ExperimentalIncrementalExecutionResults> {
154-
return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => {
155-
// If a valid execution context cannot be created due to incorrect
156-
// arguments, a "Response" with only errors is returned.
157-
const validatedExecutionArgs = validateExecutionArgs(args);
158-
159-
// Return early errors if execution context failed.
160-
if (!('schema' in validatedExecutionArgs)) {
161-
return { errors: validatedExecutionArgs };
162-
}
160+
if (!executeChannel?.hasSubscribers) {
161+
return experimentalExecuteIncrementallyImpl(args);
162+
}
163+
return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () =>
164+
experimentalExecuteIncrementallyImpl(args),
165+
);
166+
}
163167

164-
return experimentalExecuteQueryOrMutationOrSubscriptionEvent(
165-
validatedExecutionArgs,
166-
);
167-
});
168+
function experimentalExecuteIncrementallyImpl(
169+
args: ExecutionArgs,
170+
): PromiseOrValue<ExecutionResult | ExperimentalIncrementalExecutionResults> {
171+
// If a valid execution context cannot be created due to incorrect
172+
// arguments, a "Response" with only errors is returned.
173+
const validatedExecutionArgs = validateExecutionArgs(args);
174+
175+
// Return early errors if execution context failed.
176+
if (!('schema' in validatedExecutionArgs)) {
177+
return { errors: validatedExecutionArgs };
178+
}
179+
180+
return experimentalExecuteQueryOrMutationOrSubscriptionEvent(
181+
validatedExecutionArgs,
182+
);
168183
}
169184

170185
export function executeIgnoringIncremental(
171186
args: ExecutionArgs,
172187
): PromiseOrValue<ExecutionResult | ExperimentalIncrementalExecutionResults> {
173-
return maybeTraceMixed('execute', buildExecuteCtxFromArgs(args), () => {
174-
// If a valid execution context cannot be created due to incorrect
175-
// arguments, a "Response" with only errors is returned.
176-
const validatedExecutionArgs = validateExecutionArgs(args);
177-
178-
// Return early errors if execution context failed.
179-
if (!('schema' in validatedExecutionArgs)) {
180-
return { errors: validatedExecutionArgs };
181-
}
188+
if (!executeChannel?.hasSubscribers) {
189+
return executeIgnoringIncrementalImpl(args);
190+
}
191+
return traceMixed(executeChannel, buildExecuteCtxFromArgs(args), () =>
192+
executeIgnoringIncrementalImpl(args),
193+
);
194+
}
182195

183-
return executeQueryOrMutationOrSubscriptionEventIgnoringIncremental(
184-
validatedExecutionArgs,
185-
);
186-
});
196+
function executeIgnoringIncrementalImpl(
197+
args: ExecutionArgs,
198+
): PromiseOrValue<ExecutionResult | ExperimentalIncrementalExecutionResults> {
199+
// If a valid execution context cannot be created due to incorrect
200+
// arguments, a "Response" with only errors is returned.
201+
const validatedExecutionArgs = validateExecutionArgs(args);
202+
203+
// Return early errors if execution context failed.
204+
if (!('schema' in validatedExecutionArgs)) {
205+
return { errors: validatedExecutionArgs };
206+
}
207+
208+
return executeQueryOrMutationOrSubscriptionEventIgnoringIncremental(
209+
validatedExecutionArgs,
210+
);
187211
}
188212

189213
/**
@@ -244,8 +268,11 @@ export function executeSync(args: ExecutionArgs): ExecutionResult {
244268
export function executeSubscriptionEvent(
245269
validatedExecutionArgs: ValidatedExecutionArgs,
246270
): PromiseOrValue<ExecutionResult> {
247-
return maybeTraceMixed(
248-
'execute',
271+
if (!executeChannel?.hasSubscribers) {
272+
return executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs);
273+
}
274+
return traceMixed(
275+
executeChannel,
249276
buildExecuteCtxFromValidatedArgs(validatedExecutionArgs),
250277
() => executeQueryOrMutationOrSubscriptionEvent(validatedExecutionArgs),
251278
);
@@ -282,26 +309,37 @@ export function subscribe(
282309
): PromiseOrValue<
283310
AsyncGenerator<ExecutionResult, void, void> | ExecutionResult
284311
> {
285-
return maybeTraceMixed('subscribe', buildExecuteCtxFromArgs(args), () => {
286-
// If a valid execution context cannot be created due to incorrect
287-
// arguments, a "Response" with only errors is returned.
288-
const validatedExecutionArgs = validateExecutionArgs(args);
289-
290-
// Return early errors if execution context failed.
291-
if (!('schema' in validatedExecutionArgs)) {
292-
return { errors: validatedExecutionArgs };
293-
}
312+
if (!subscribeChannel?.hasSubscribers) {
313+
return subscribeImpl(args);
314+
}
315+
return traceMixed(subscribeChannel, buildExecuteCtxFromArgs(args), () =>
316+
subscribeImpl(args),
317+
);
318+
}
294319

295-
const resultOrStream = createSourceEventStreamImpl(validatedExecutionArgs);
320+
function subscribeImpl(
321+
args: ExecutionArgs,
322+
): PromiseOrValue<
323+
AsyncGenerator<ExecutionResult, void, void> | ExecutionResult
324+
> {
325+
// If a valid execution context cannot be created due to incorrect
326+
// arguments, a "Response" with only errors is returned.
327+
const validatedExecutionArgs = validateExecutionArgs(args);
296328

297-
if (isPromise(resultOrStream)) {
298-
return resultOrStream.then((resolvedResultOrStream) =>
299-
mapSourceToResponse(validatedExecutionArgs, resolvedResultOrStream),
300-
);
301-
}
329+
// Return early errors if execution context failed.
330+
if (!('schema' in validatedExecutionArgs)) {
331+
return { errors: validatedExecutionArgs };
332+
}
333+
334+
const resultOrStream = createSourceEventStreamImpl(validatedExecutionArgs);
335+
336+
if (isPromise(resultOrStream)) {
337+
return resultOrStream.then((resolvedResultOrStream) =>
338+
mapSourceToResponse(validatedExecutionArgs, resolvedResultOrStream),
339+
);
340+
}
302341

303-
return mapSourceToResponse(validatedExecutionArgs, resultOrStream);
304-
});
342+
return mapSourceToResponse(validatedExecutionArgs, resultOrStream);
305343
}
306344

307345
/**

src/language/parser.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { Maybe } from '../jsutils/Maybe.js';
33
import type { GraphQLError } from '../error/GraphQLError.js';
44
import { syntaxError } from '../error/syntaxError.js';
55

6-
import { parseChannel } from '../diagnostics.js';
6+
import { parseChannel, shouldTrace } from '../diagnostics.js';
77

88
import type {
99
ArgumentCoordinateNode,
@@ -134,10 +134,9 @@ export function parse(
134134
source: string | Source,
135135
options?: ParseOptions,
136136
): DocumentNode {
137-
if (!parseChannel?.hasSubscribers) {
138-
return parseImpl(source, options);
139-
}
140-
return parseChannel.traceSync(() => parseImpl(source, options), { source });
137+
return shouldTrace(parseChannel)
138+
? parseChannel.traceSync(() => parseImpl(source, options), { source })
139+
: parseImpl(source, options);
141140
}
142141

143142
function parseImpl(

src/validation/validate.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { assertValidSchema } from '../type/validate.js';
1212

1313
import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo.js';
1414

15-
import { validateChannel } from '../diagnostics.js';
15+
import { shouldTrace, validateChannel } from '../diagnostics.js';
1616

1717
import { specifiedRules, specifiedSDLRules } from './specifiedRules.js';
1818
import type { SDLValidationRule, ValidationRule } from './ValidationContext.js';
@@ -63,13 +63,12 @@ export function validate(
6363
rules: ReadonlyArray<ValidationRule> = specifiedRules,
6464
options?: ValidationOptions,
6565
): ReadonlyArray<GraphQLError> {
66-
if (!validateChannel?.hasSubscribers) {
67-
return validateImpl(schema, documentAST, rules, options);
68-
}
69-
return validateChannel.traceSync(
70-
() => validateImpl(schema, documentAST, rules, options),
71-
{ schema, document: documentAST },
72-
);
66+
return shouldTrace(validateChannel)
67+
? validateChannel.traceSync(
68+
() => validateImpl(schema, documentAST, rules, options),
69+
{ schema, document: documentAST },
70+
)
71+
: validateImpl(schema, documentAST, rules, options);
7372
}
7473

7574
function validateImpl(

0 commit comments

Comments
 (0)