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

Using variables destroys performance #2316

Closed
wooque opened this issue Jun 2, 2019 · 5 comments
Closed

Using variables destroys performance #2316

wooque opened this issue Jun 2, 2019 · 5 comments
Assignees
Labels
c/docs Related to docs e/quickfix can be wrapped up in few hours p/high candidate for being included in the upcoming sprint

Comments

@wooque
Copy link

wooque commented Jun 2, 2019

Query without variables:

{
  user_news(where: {user_id: {_eq: 1}, status: {_eq: 0}}, limit: 10, order_by: {id: desc}) {
    id
  }
}

Throughput: 3800 req/s

Query with variables:

query($user:Int, $status: Int){
  user_news(where: {user_id: {_eq: $user}, status: {_eq: $status}}, limit: 10, order_by: {id: desc}) {
    id
  }
}

Variables:

{
  "user":1,
  "status":0
}

Throughput: 980 req/s

In more complex example where I first noticed this performance drop it went from ~1500req/s to ~250req/s.

@0x777
Copy link
Member

0x777 commented Jun 3, 2019

Use non nullable variables. The query plan caching kicks in when all the variables are non-nullable scalars.

query($user:Int!, $status: Int!){
  user_news(where: {user_id: {_eq: $user}, status: {_eq: $status}}, limit: 10, order_by: {id: desc}) {
    id
  }
}

@0x777
Copy link
Member

0x777 commented Jun 3, 2019

Also, if you can spare more than one core to graphql-engine, you can set GHCRTS environment variable to -N (uses all available cores, you can restrict it by -N<n>, like -N2).

@wooque
Copy link
Author

wooque commented Jun 3, 2019

Yep, non nullable variables brought it to 90+% of non variable query performance.
This should really be in documentation because it will byte a lot of people, I didn't even know that variables could be nullable, let alone that there is big difference in performance.

The trick with with GHCRTS also improved performance for quite a bit, I'm surprised it's not default

@0x777
Copy link
Member

0x777 commented Jun 4, 2019

This should really be in documentation because it will byte a lot of people, I didn't even know that variables could be nullable, let alone that there is big difference in performance.

We'll add this to documentation.

The trick with GHCRTS also improved performance for quite a bit, I'm surprised it's not default

Historical reasons. We'll set it as the default in the next release.

@wooque
Copy link
Author

wooque commented Jun 4, 2019

Thanks. I would leave this open as reminder

@rikinsk-zz rikinsk-zz added the c/docs Related to docs label Jun 4, 2019
@0x777 0x777 self-assigned this Jun 11, 2019
@marionschleifer marionschleifer added p/high candidate for being included in the upcoming sprint e/quickfix can be wrapped up in few hours labels Sep 9, 2019
stevefan1999-personal pushed a commit to stevefan1999-personal/graphql-engine that referenced this issue Sep 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/docs Related to docs e/quickfix can be wrapped up in few hours p/high candidate for being included in the upcoming sprint
Projects
None yet
Development

No branches or pull requests

4 participants