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

Performance problem on large datasets #244

Closed
ahocquard opened this issue Feb 9, 2018 · 13 comments
Closed

Performance problem on large datasets #244

ahocquard opened this issue Feb 9, 2018 · 13 comments

Comments

@ahocquard
Copy link

ahocquard commented Feb 9, 2018

Hello,

It seems I have some performance problems on large datasets with the library.

My request is:

{
  families(codes: [
     ... 100 family codes
  ]) {
    code,
    attributes {
      code
    }
  }
}

Inside each families, I have 100 attributes.
It takes more than 1 seconds to respond. It seems that GraphQL implementation call resolveField for each field, and that's it's pretty slow.

And when I want to display all properties in a family, it takes more than 6 seconds.
In this case, the rest API only takes 2 seconds.
Theoretically, rest API should be slower because with GraphQL, I use the dataloader, reducing drastically the number of requests to the database.

I tried to figure out the performance of the library by using your phpbench tests, but resolving 10000 fields takes only 200 ms with the generated schema.

So, I'm wondering if I did something badly or if GraphQL is pretty slow on large datasets.
I saw that this problem is common on different languages (graphql/graphql-js#723).

I can provide blackfire profile if you want.

Thanks for any advice.

@mcg-web
Copy link
Collaborator

mcg-web commented Feb 9, 2018

Hi,
have you try using variables instead of literal:

query ($codes: [String!]!){
  families(codes: $codes) {
    code,
    attributes {
      code
    }
  }
}

variables:

{"codes": [
     ... 100 family codes
]}

This should ease the parser work.

@ahocquard
Copy link
Author

Hello,

Unfortunately, it does not improve the performance.
It seems that it's due to the number fo fields to resolve (100 families with 100 attributes with 3 fields = 30000). GraphQL does some extra-work to handle a lot of unused thing in my case (directive and so on). I'm still pretty new with the technology though.

Do you have some feedback with other projects about such performance problems?

@vladar
Copy link
Member

vladar commented Feb 12, 2018

GraphQL is a better fit for highly structured content (when you may request lots of different subsets of data). It is not the best fit for situations when you only query for a list with a bunch of flat attributes (unfiltered).

Custom REST endpoints will always outperform GraphQL in such scenario because well, you don't need most of the features of GraphQL for such queries!

As for performance issues - there is indeed an overhead for each field, we do have plans to adjust common scenarios to use short evaluation paths and memoization, but no ETA for this feature.

Actually, it is better to watch after graphql-js thread you mentioned, as they also experiment with such optimizations and we'll likely follow the same route they choose eventually.

But also make sure your data layer is optimized as (usually) it is the bottleneck. Say how many requests to the underlying store do you make? How is your internal data structured, etc?

@ahocquard
Copy link
Author

ahocquard commented Feb 12, 2018

Actually, I'm just doing 4 SQL requests. It costs "only" 260 ms when requesting all the data of the family entity. But it takes more than 6 seconds to return everything. Most of the time is spent by GraphQL resolver.

My SQL requests use the powerfulness of the newjson_arrayagg operator of Mysql 8.0.
The result is that hydration is not costly at all (almost all the job is done by Mysql). I'm satisfied with it, as it's outperform hydration done by Doctrine (for example).

Combined it with a dataloader, and results are really good.

Family REST endpoint, on the other side, can trigger more than 300 SQL requests for such families.
But REST it's still 3 times faster.

Of course, when I request only some fields, GraphQL outperform REST.

To be honest, I would bet on an improvement by 3 or 4x with GraphQL, as the SQL requests + hydration is highly optimized.

Thanks for reply.
Looking forward for improvements!

@vladar
Copy link
Member

vladar commented Feb 12, 2018

Do you use default field resolver? It is hard to say anything without seeing some code (or better) reproducible example (since data layer is not the case - I guess we can just stub it with arrays to reproduce reliably).

@ahocquard
Copy link
Author

ahocquard commented Feb 12, 2018

It's a proof of concept, but you can access code is in this bundle: https://github.com/ahocquard/pim-community-dev/blob/poc-the-poc/src/Pim/Bundle/ResearchBundle/PimResearchBundle.php

It's not related to any other code of the whole software at this moment, so you can focus on it.

I'm using dedicated resolverresolveField for each type.

I will try to provide an example with the same volume of data and with fake repositories.

@vladar
Copy link
Member

vladar commented Mar 26, 2018

I tried to reproduce this issue on your codebase, but couldn't. Check out https://github.com/vladar/graphql-perf-debug and run it on your machine.

Mine laptop returns about 0.43 second to execute the query against an in-memory store (with dataloader, etc) on PHP 7.2 (but make sure to run it several times to populate PHP cache)

So result contains 100 entries, each containing 100 attributes (10000 total). Maybe I've missed something, so feel free to adjust the code to reproduce.

@vladar
Copy link
Member

vladar commented Mar 26, 2018

And make sure you have xdebug (and other similar tools) disabled when testing.

@vladar
Copy link
Member

vladar commented Apr 8, 2018

Closing this for now. But if you have fresh details, feel free to re-open!

@vladar vladar closed this as completed Apr 8, 2018
@ahocquard
Copy link
Author

ahocquard commented Apr 15, 2018

Hello,

Really sorry for the delay, I was pretty busy.
I really appreciated the test you did and it deserves some investigations on my side ;)

I had the same results as yours with https://github.com/vladar/graphql-perf-debug (~400ms).
I tested with my codebase as well (Symfony, DI, etc), by creating fake objects, and I had the same results.

The extra overhead of the SQL requests is about 250 ms.
So, I would expect a response in less than 700 ms (GraphQL 400 ms + 250 ms of sql request + 50 ms of extra-stuff). But I got 1 second. It means that GraphQL rendering costs about 700ms and not 400 ms as tested before.

And I found the reason: the test you did was creating the same attributes for all the families.
And GraphQL seems to already have an optimisation in such scenario.

If you have completely different attribute codes for each family, the result is pretty bad: ~6s5 on my computer (meaning 10000 different attributes).
If you have different attribute codes among 600 attributes (the real condition that I initially tested), it takes ~750ms. So, it's more or less what is expected.

You can test it here:
https://github.com/ahocquard/graphql-perf-debug/

In conclusion, GraphQL implementation is pretty fast for most of the needs, but it can be pretty slow on large datasets, and it depends of the distributivity of the data.

@vladar
Copy link
Member

vladar commented Apr 15, 2018

Thanks for the further details! Actually, I did some profiling and the bottleneck is not graphql, but dataloader.

Those lines are to blame: https://github.com/overblog/dataloader-php/blob/master/src/CacheMap.php#L63-L71

It makes any cache operation O(N) vs O(1) with obvious consequences. I did a quick change locally to just use array key vs looping over all cache entries and the total time for your test_2.php had dropped from 6s to ~0.7s

Even here the overhead above 0.4s is caused by other tools, not graphql. There is no special caching right now on the graphql side of things. So for graphql it is irrelevant if you have 100 same attributes or different.

@mcg-web Can you share some thoughts on why you guys use looping vs lookup in the lines above and can you change this?

@vladar vladar reopened this Apr 15, 2018
@mcg-web
Copy link
Collaborator

mcg-web commented Apr 15, 2018

@vladar thank you for the feedback. We should give this a deeper look but we'll fix it a fast as possible 👍.

@vladar
Copy link
Member

vladar commented Apr 24, 2018

So this is actually not a problem of this lib, closing

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

No branches or pull requests

3 participants