search

graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.1k stars 2.03k forks source link

perf: optimize completion loops #4053

Closed yaacovCR closed 6 months ago

yaacovCR commented 7 months ago

by hoisting streamUsage checks out of the loop, requires introducing two versions of the looping functions

depends on #4052

netlify[bot] commented 7 months ago

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
Latest commit 9ba1510d26666dc45f1e68e472fb1a66e674f682
Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/663c382587b6b70008b12df2
Deploy Preview https://deploy-preview-4053--compassionate-pike-271cb3.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions[bot] commented 7 months ago

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands Please post this commands in separate comments and only one per comment: * `@github-actions run-benchmark` - Run benchmark comparing base and merge commits for this PR * `@github-actions publish-pr-on-npm` - Build package from this PR and publish it on NPM
yaacovCR commented 7 months ago

image

yaacovCR commented 6 months ago

After offline discussion with @robrichard i am closing this one for now as not worth the extra slight performance bump

by our very synthetic benchmarks, in the very long synchronous list case we’re already faster than the first 17 alpha release and in the asynchronous case we’re still slower either way secondary to incremental delivery, so the small difference from this PR doesn’t really change our performance story overall

in the short term, but not immediate future we can think about #4043 as the real path away from the performance penalty involved with incremental delivery

although the current plan is not to proceed with that immediately as during the pre-and presumably immediate post release. After incremental delivery, there is significant value in having our reference and limitation be more aligned with the specification changes.

4043 add an optimization that diverges somewhat from the specification, and so we may adopt it within the reference implementation, but the thought is at least not to do so immediately