Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: simplify and optimize ElasticGraph::GraphQL::Schema. #31

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

myronmarston
Copy link
Collaborator

  • Avoid calling ::GraphQL::Schema#types repeatedly. As reported in Performance regression in 2.4: Schema#types is much slower to access repeatedly  rmosolgo/graphql-ruby#5154, accessing #types repeatedly can be quite slow (at least on 2.4.0 - 2.4.2). While it's been optimized in 2.4.3, there's no need to access it more than once. Previously, we accessed it in #lookup_type_by_name; instead we can just use the type objects as we iterate over ::GraphQL::Schema#types a single time.
  • Remove unused #defined_types method.
  • Inline the old #lookup_type_by_name logic into #type_named.

- Avoid calling `::GraphQL::Schema#types` repeatedly. As reported in rmosolgo/graphql-ruby#5154,
  accessing `#types` repeatedly can be quite slow (at least on 2.4.0 - 2.4.2). While it's been
  optimized in 2.4.3, there's no need to access it more than once. Previously, we accessed it
  in `#lookup_type_by_name`; instead we can just use the type objects as we iterate over
  `::GraphQL::Schema#types` a single time.
- Remove unused `#defined_types` method.
- Inline the old `#lookup_type_by_name` logic into `#type_named`.
@myronmarston myronmarston force-pushed the myron/upgrade-graphql-2.4/schema-prep1 branch from c748940 to 571c2fd Compare November 12, 2024 07:36
@myronmarston myronmarston changed the title Refactor: simplify and optimiize ElasticGraph::GraphQL::Schema. Refactor: simplify and optimize ElasticGraph::GraphQL::Schema. Nov 12, 2024
Copy link
Collaborator

@BrianSigafoos-SQ BrianSigafoos-SQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@myronmarston myronmarston merged commit d9ef89b into main Nov 12, 2024
10 checks passed
@myronmarston myronmarston deleted the myron/upgrade-graphql-2.4/schema-prep1 branch November 12, 2024 16:10
myronmarston added a commit that referenced this pull request Nov 12, 2024
I removed it in #31 because ElasticGraph didn't rely on it or need it.
However, I discovered that Block has an internal extension library that
relies on it, and it's a generally useful API to offer for extension
libraries. This restores it (but with a simplified implementation).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants