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

Proposal: Faster Algorithm For OverlappingFieldsCanBeMerged #2185

Open
acceleratesage opened this issue Sep 19, 2019 · 2 comments
Open

Proposal: Faster Algorithm For OverlappingFieldsCanBeMerged #2185

acceleratesage opened this issue Sep 19, 2019 · 2 comments

Comments

@acceleratesage
Copy link

acceleratesage commented Sep 19, 2019

At XING we are using the Sangria implementation of GraphQL. The implementation of the OverlappingFieldsCanBeMerged validation in Sangria follows the one in this reference implementation. We experienced performance problems with it related to the number of fragments. After we invested some time to solve it, I came up with a better algorithm, that we are using now.

  • Here is the description of the algorithm.
  • And here is our PR for Sangria.

The proposal would be to use this algorithm in the reference implementation. Below is an example benchmark graph showing the performance improvement for multiple fragments in the same selection set.

2019-09-16 NoOverlapFrag

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Sep 19, 2019

@SimonAdameit Thanks a lot for sharing 👍 Its super useful ATM we preparing for TS conversion so we in a feature freeze state but it would be at the top queue after conversion is finished.
Also, I think we need to use this opportunity and address edge cases raised in #1497 at the same time.

@yaacovCR
Copy link
Contributor

but it would be at the top queue after conversion is finished.
Also, I think we need to use this opportunity and address edge cases raised in #1497 at the same time.

with TS conversion finished, this should be in the news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants