perf(gqlize): measure the request path, and change nothing - #63
Merged
Conversation
The plan's last group was "optimise the resolution path". This measures it first,
and the measurement says not to.
`bench-artifact.ts` covers building and loading a schema, which happens once per
process. There was nothing for the other half — executing a query, which happens
per request and is where a regression actually hurts. `bench-resolve.ts` fills
that gap, reusing the existing `syntheticDefinitions` generator and reporting
percentiles rather than a mean: a resolution path allocates, so the distribution
has a GC tail and a mean hides exactly what an optimisation is supposed to move.
The number that decides it — the same read, as a full GraphQL request and as a
bare `Model.findAll` over the same rows:
raw sequelize findAll p50 1.474 ms
full graphql request p50 1.777 ms
The database is 83% of the request. Everything this repo owns — resolution
engine, graphql execution, serialisation — is the remaining 17%, and that is the
most favourable framing available: sqlite runs in-process, so a real database
over a socket makes the engine's share smaller still. A change that made the
whole path 20% faster would move a request by about 3%.
So nothing is optimised here, and that is the deliverable rather than an
omission. The harness and the number are what a future change needs in order to
justify itself.
Two things were checked while looking, and needed no change:
- The copy-on-write guards added in 7.0.0-beta.11 do allocate nothing in the
steady state. `expandComputedOrder` returns its input by identity — the same
args object, and the same nested `include` array — whenever the ordering names
a real column rather than a computed one, which is every ordinary request.
That had been an assertion in a comment; it is now verified.
- `getFields` is already memoised per model via `getMetaObj`, and
`getDefinition` is a plain object index. Neither repeats work per field or per
row, which is what their call counts (39 and 36 sites) might otherwise suggest.
The README's performance section gains the per-request table beside the
per-process one, and says plainly that the queries and indexes are where the
time is.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fGumVzMfMXDZ5vS7PJGDW
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.
Group 5. The plan's last group was "optimise the resolution path". This measures it first, and the measurement says not to.
The gap
bench-artifact.tscovers building and loading a schema — once per process. There was nothing for the other half: executing a query, which happens per request and is where a regression actually hurts.bench-resolve.tsfills it, reusing the existingsyntheticDefinitionsgenerator. It reports percentiles, not a mean — a resolution path allocates, so the distribution has a GC tail and a mean hides exactly what an optimisation is supposed to move.The number that decides it
The same read, as a full GraphQL request and as a bare
Model.findAllover the same rows:The database is 83% of the request. Everything this repo owns — resolution engine, graphql execution, serialisation — is the other 17%.
And that's the most favourable framing available: sqlite runs in-process. A real database over a socket makes the engine's share smaller, not larger.
So a change making the whole path 20% faster would move a request by about 3%.
Nothing is optimised here, and that's the deliverable rather than an omission — the harness and the number are what a future change needs in order to justify itself.
Two assumptions checked
Both held, so neither needed a change:
expandComputedOrderreturns its input by identity — same args object, same nestedincludearray — whenever the ordering names a real column rather than a computed one, which is every ordinary request. That had been an assertion in a comment; it's now verified.getFieldsis already memoised per model viagetMetaObj, andgetDefinitionis a plain object index. Neither repeats work per field or per row, which their call counts (39 and 36 sites) might otherwise suggest.Also
The README's performance section gains the per-request table beside the per-process one, and says plainly that queries and indexes are where the time is.
🤖 Generated with Claude Code
https://claude.ai/code/session_019fGumVzMfMXDZ5vS7PJGDW