What
graphql.NewScalarFieldContext's fast path (added in #4086 for argument-less
scalar/enum fields) is called from every generated fieldContext_* function
for such a field, and each call builds a fresh errors.New(...):
return graphql.NewScalarFieldContext(objectName, field, isMethod, isResolver,
errors.New("field of type String does not have child fields"))That error is only ever read if a query illegally selects sub-fields on a
scalar — essentially never. It's still built on every call, because it's a
function argument and Go evaluates it eagerly regardless of whether the
callee uses it.
Measured cost
Benchmarked the generated fieldContext_Todo_id in _examples/todo,
same benchmark before/after a fix, results forced into a package-level sink
so the compiler can't optimize the call away:
before: 48.36 ns/op 184 B/op 3 allocs/op
after: 40.69 ns/op 168 B/op 2 allocs/op
One allocation removed — nothing else in the call chain changed. This is
per field, per resolution, unconditionally: a schema with 300 scalar/enum
leaf fields pays 300 avoidable allocations per operation that touches all of
them, regardless of query shape or whether anything actually errors.
Is this a regression?
Yes, precisely traceable to #4086. Before that PR, every fieldContext_*
function — scalar or not, with args or without — used a single code path
that built this same error inside the Child closure, i.e. lazily,
evaluated only if Child is actually invoked. #4086 split that into a fast
path (argument-less scalar/enum fields) and a verbose fallback (fields with
args), to cut ~8 lines of generated code per field. The fallback kept the
lazy pattern; the fast path — the common case — didn't, because the error
became a plain eagerly-evaluated argument instead of closure-internal code.
#4086's own description claims "zero runtime performance impact," measured
in generated-line-count terms; this doesn't hold for allocations.
Proposed fix
Since the message depends only on the GraphQL type name, and there's a
small, fixed set of those per schema, generate one shared error value per
distinct scalar/enum type name (mirroring the existing
UniqueChildFieldTypes / shared childFields_* pattern from #4086 itself),
and have the fast-path template reference it instead of calling errors.New
per field. NewScalarFieldContext's public signature is unchanged — this
only changes what gets passed into it, so it's non-breaking.
Implemented, tested (go test ./codegen/... ./graphql/... ./plugin/...,
golangci-lint), and regenerated (root module + _examples) in draft PR
#4293 — filing this proposal per CONTRIBUTING.md before asking for review,
since it touches generated output for every scalar/enum field in every
schema. Open to feedback on approach (e.g. where the shared vars should live)
before marking it ready.
What
graphql.NewScalarFieldContext's fast path (added in #4086 for argument-lessscalar/enum fields) is called from every generated
fieldContext_*functionfor such a field, and each call builds a fresh
errors.New(...):That error is only ever read if a query illegally selects sub-fields on a
scalar — essentially never. It's still built on every call, because it's a
function argument and Go evaluates it eagerly regardless of whether the
callee uses it.
Measured cost
Benchmarked the generated
fieldContext_Todo_idin_examples/todo,same benchmark before/after a fix, results forced into a package-level sink
so the compiler can't optimize the call away:
One allocation removed — nothing else in the call chain changed. This is
per field, per resolution, unconditionally: a schema with 300 scalar/enum
leaf fields pays 300 avoidable allocations per operation that touches all of
them, regardless of query shape or whether anything actually errors.
Is this a regression?
Yes, precisely traceable to #4086. Before that PR, every
fieldContext_*function — scalar or not, with args or without — used a single code path
that built this same error inside the
Childclosure, i.e. lazily,evaluated only if
Childis actually invoked. #4086 split that into a fastpath (argument-less scalar/enum fields) and a verbose fallback (fields with
args), to cut ~8 lines of generated code per field. The fallback kept the
lazy pattern; the fast path — the common case — didn't, because the error
became a plain eagerly-evaluated argument instead of closure-internal code.
#4086's own description claims "zero runtime performance impact," measured
in generated-line-count terms; this doesn't hold for allocations.
Proposed fix
Since the message depends only on the GraphQL type name, and there's a
small, fixed set of those per schema, generate one shared error value per
distinct scalar/enum type name (mirroring the existing
UniqueChildFieldTypes/ sharedchildFields_*pattern from #4086 itself),and have the fast-path template reference it instead of calling
errors.Newper field.
NewScalarFieldContext's public signature is unchanged — thisonly changes what gets passed into it, so it's non-breaking.
Implemented, tested (
go test ./codegen/... ./graphql/... ./plugin/...,golangci-lint), and regenerated (root module +_examples) in draft PR#4293 — filing this proposal per CONTRIBUTING.md before asking for review,
since it touches generated output for every scalar/enum field in every
schema. Open to feedback on approach (e.g. where the shared vars should live)
before marking it ready.