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

Significant performance difference of GraphQL.execute() between 19 and 21~22 version #3586

Open
FutureGadget opened this issue May 3, 2024 · 13 comments

Comments

@FutureGadget
Copy link

FutureGadget commented May 3, 2024

The average time take of executeQuery() test shows significant difference between version 19.11 and 21.0.
https://github.com/graphql-java/graphql-java/blob/v21.0/src/test/java/benchmark/BenchMark.java#L54
https://github.com/graphql-java/graphql-java/blob/v19.11/src/test/java/benchmark/BenchMark.java#L55

I've tested it by changing the NUMBER_OF_FRIENDS value to 10000, which is 10 times of the default setting in the benchmark test code.

Query used in the benchmark code

{
  hero {
    name
    friends {
      name
      friends {
        name
      }
    }
  }
}

Object used in the benchmark code

{
    "name": "r2d2",
    "friends": [
        {
            "name": "friend1",
            "friends":[],
        },
        {
            "name": "friend2",
            "friends":[],
        },
        ...
        // 10000 friends
    ]
}

Result by graphql-java versions

version result
19.11 113.288ms
20 111.996ms
21.0 40.926ms
21.2 41.718ms
22 6.430ms

Is it possible to apply the performance improvements that were done in 21.0 version to the 19.11 version?

I think the difference is not from java version (graphql-java 21.0 is using java 17) since the improvement is quite huge.

Thank you!

@FutureGadget FutureGadget changed the title Significant performance difference of GraphQL.execute() between 19 and 20 version Significant performance difference of GraphQL.execute() between 19 and 21~22 version May 3, 2024
@dondonz
Copy link
Member

dondonz commented May 3, 2024

I think the difference is not from java version (graphql-java 21.0 is using java 17) since the improvement is quite huge.

Hello, to nitpick and clarify, graphql-java 21.0 is using Java 11. We're not yet adopting Java 17 as the baseline.

Thanks for the data points it's great to see others validating the huge effort that's recently gone into performance. There's even more performance improvements included in v22.0.

We don't usually backport performance improvements. We typically backport only bugfixes or security updates, and only for a limited time as per our release policy. https://www.graphql-java.com/blog/release-policy

Thanks for asking though and thanks for the data! Hopefully you are able to upgrade to a later version of GraphQL Java

@FutureGadget
Copy link
Author

FutureGadget commented May 3, 2024

@dondonz Thanks for your response. Okay, we are putting many efforts to use springboot 3 so that we can use dgs 8.x version. Also, thanks for all the efforts for the community having done to make these huge performance improvements.

@bbakerman
Copy link
Member

The numbers look too good to be true but benchmarks can be like that. Its all depends on the machine and anything else running on it.

I ran them on my laptop and I get

==========
1000 friends

V19x

Benchmark                                    Mode  Cnt   Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  86.025 ± 2.867  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  11.378 ± 0.297  ms/op

V22x

Benchmark                                    Mode  Cnt    Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  141.957 ± 5.981  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15    7.028 ± 0.113  ms/op


==========
10 000 friends

v19x

Benchmark                                    Mode  Cnt    Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15    7.435 ± 0.309  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  131.635 ± 5.389  ms/op

v22x


Benchmark                                    Mode  Cnt   Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  12.565 ± 0.326  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  76.644 ± 2.711  ms/op

So much better but not in the same order of magnitude you have above

So v22 is better but not at the same level you have above

I also ran them on different JVM because I did it from source and not from a seperate project. It might be worth create a project that depends on the release version rather than from source

Is it possible to apply the performance improvements that were done in 21.0 version to the 19.11 version?

The answer is no in terms of code policy and now in terms of being able to even attempt that. There is no magic one thing that makes a difference. Its a series of fixes and changes that compound over versions.

@FutureGadget
Copy link
Author

FutureGadget commented May 4, 2024

@bbakerman Thanks for testing it again. The performance improvement is truly unbelievable.
I did it on M1 Macbook pro with 10 cores, and I did it again on M2 Macbook pro with 12 cores.

version jvm hw result
19.11 azul-1.8 M1 Macbook Pro 10 core 113.288ms
19.11 azul-1.8 M2 Macbook Pro 12 core 111.083ms
20 azul-1.8 M1 Macbook Pro 10 core 111.996ms
21.0 corretto-17 M1 Macbook Pro 10 core 40.926ms
21.2 corretto-17 M1 Macbook Pro 10 core 41.718ms
22 corretto-17 M1 Macbook Pro 10 core 6.430ms
22 corretto-17 M2 Macbook Pro 12 core 5.703ms

I also tested 1000 friends case with M2 Macbook Pro 12 core. It shows almost exactly 1/10 of the 10000 case, which seems natural.

Result "benchmark.SimpleQueryBenchmark.benchMarkSimpleQueriesAvgTime":
  0.595 ±(99.9%) 0.054 ms/op [Average]
  (min, avg, max) = (0.554, 0.595, 0.632), stdev = 0.032
  CI (99.9%): [0.541, 0.649] (assumes normal distribution)

The answer is no in terms of code policy and now in terms of being able to even attempt that. There is no magic one thing that makes a difference. Its a series of fixes and changes that compound over versions.

I understand this. I just had a little hope about a single significant performance bottleneck point that can be back ported to the previous versions which will benefit a lot of users who are still on Spring 5 (Springboot 2.x) and using DGS framework version 5.x.

We are now putting a lot of effort on upgrading our application's Springboot version to 3.2.5.
Thank you for taking a look into my somewhat non-sensical question.

@bbakerman
Copy link
Member

bbakerman commented May 6, 2024

So you massively nerd sniped me - I wrote a gradle script that takes the grpahql-java version as env variable and I also ran it on different jvms


declare -A jvms=( ["11"]="11.0.23-amzn" ["17"]="17.0.1.12.1-amzn" ["21"]="21.0.3-amzn")

echo "Lets test!"
echo "=================="

for jV in "11" "17" "21"; do

  jvmName="${jvms[$jV]}"
  sdk use java $jvmName
  java -version

  for gV in "19.0" "20.0" "21.0" "22.0"; do
    echo "=================="
    echo "Testing on $jvmName with graphql-java $gV"
    echo "=================="

    export GRAPHQL_VERSION=$gV; gw clean build run
  done
done

Which then runs graphql-java versions on 3 different JVMs. And while the newer graphql-java version is clearly faster, the jvm matters as well

for example on the JDK 11 (Amazon Correto) with graphql-java 19.0

Benchmark                                    Mode  Cnt   Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  11.972 ± 0.408  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  85.277 ± 1.164  ms/op

versus the same code but on the latest JDK 21 (Amazon Correto) with graphql-java 19.0 (same code different JVMs)

Benchmark                                    Mode  Cnt   Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  13.371 ± 0.164  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  75.261 ± 1.233  ms/op

eg a 11.7% decrease in ms/op on the newer JVMs. So JVMs matter - but yeah the better code matters the most

JDK 21 (Amazon Correto) with graphql-java 22.0

Benchmark                                    Mode  Cnt   Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  23.170 ± 0.749  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  43.761 ± 1.701  ms/op

eg a 42.6% decrease in ms/op on the same JVM

The benchmark is an in memory set of data and hence all values are in memory as well.

Now the thing is graphql-java v22 did something that suits this benchmark. Its removed the CompleteableFuture wrapping that happens on values. Instead values that are a not an async promise are not wrapped and used directly. This reduces memory pressure and GC invocation times (albeit it complicates the code)

We can see that in the v21 comparison

JDK 21 (Amazon Correto) with graphql-java 21.0

Benchmark                                    Mode  Cnt   Score   Error  Units
BenchMark.benchMarkSimpleQueriesThroughput  thrpt   15  13.582 ± 0.142  ops/s
BenchMark.benchMarkSimpleQueriesAvgTime      avgt   15  71.558 ± 0.999  ms/op

a 39.4% decrease in ms/op compared to v22. So I suspect the CF code change AND that fact that the benchmark is biased towards in memory data accounts for the way better performance.

That said I don't get your numbers of 41.718ms -> 5.703ms which are even more extreme 87.8%` decrease in ms/op.

Amazon.com-Inc.-21.0.3-with-graphql-java-v20.0.txt
Amazon.com-Inc.-21.0.3-with-graphql-java-v21.0.txt
Amazon.com-Inc.-21.0.3-with-graphql-java-v22.0.txt
Amazon.com-Inc.-11.0.23-with-graphql-java-v19.0.txt
Amazon.com-Inc.-11.0.23-with-graphql-java-v20.0.txt
Amazon.com-Inc.-11.0.23-with-graphql-java-v21.0.txt
Amazon.com-Inc.-11.0.23-with-graphql-java-v22.0.txt
Amazon.com-Inc.-17.0.1-with-graphql-java-v19.0.txt
Amazon.com-Inc.-17.0.1-with-graphql-java-v20.0.txt
Amazon.com-Inc.-17.0.1-with-graphql-java-v21.0.txt
Amazon.com-Inc.-17.0.1-with-graphql-java-v22.0.txt
Amazon.com-Inc.-21.0.3-with-graphql-java-v19.0.txt

@FutureGadget
Copy link
Author

Thanks for the update. For your reference, I've recorded video evidence to support my previous statement.
You can find it here: https://github.com/graphql-java/graphql-java/assets/11131630/60f257e5-003f-46da-a93e-948be936173e

Sorry for not digging into what makes the difference.

@bbakerman
Copy link
Member

ps.... just in case you thought nerd sniping was a bad term

https://xkcd.com/356/

@bbakerman
Copy link
Member

I think I can see a major difference in the Benchmark across versions

The older one has

        return GraphQL.newGraphQL(graphQLSchema)
                .instrumentation(new TracingInstrumentation())
                .build();

while the later version of that benchmark (which got renamed at some point) took out the TracingInstrumentation - so the later versions will inherently run faster and hence its comparing apples with oranges.

When I ran my numbers above I had TracingInstrumentation in place for all versions. So they compare well to each other.

@FutureGadget
Copy link
Author

@bbakerman Wow.. that's the exact reason. Sorry for the confusion.
I've tested it by removing the TracingInstrumentation and got the following results.

v19.11
jdk 8
Result "benchmark.BenchMark.benchMarkSimpleQueriesAvgTime":
54.854 ±(99.9%) 0.880 ms/op [Average]
(min, avg, max) = (53.781, 54.854, 57.040), stdev = 0.823
CI (99.9%): [53.974, 55.733] (assumes normal distribution)

The reason why going from 20 to 21 showed dramatic improvement was because the TracingInstrumentation is removed from the benchmark.

But still, the 22 version shows drastic improvement.
Do you have specific reason you add the TracingInstrumentation when you run the 22 version?
I think you will get the similar results if you remove the tracing instrumentation.

@dfa1
Copy link
Contributor

dfa1 commented May 14, 2024

@bbakerman @FutureGadget I see also big gains (20% faster) in our services. Especially when a DataFetcher returns a CompletableFuture<SomeBigObjectInMemory> that requires no further async operations.

@dfa1
Copy link
Contributor

dfa1 commented May 14, 2024

The reason why going from 20 to 21 showed dramatic improvement was because the TracingInstrumentation is removed from the benchmark.

Oh yes, I remember removing it last year in order to have less noise in the profiler.

But still, the 22 version shows drastic improvement. Do you have specific reason you add the TracingInstrumentation when you run the 22 version? I think you will get the similar results if you remove the tracing instrumentation.

I guess to avoid comparing apples to oranges?

@cyang20240523
Copy link

hi Currently I am using graphql-java versions 22; and my jdk version is 17, I have tried jdk11, but still not working.
Can anyone be so kind enough to help me solve this problem?
The error content is as follows:
Error:(9, 15) java: cannot access graphql.ExecutionResult
Wrong class file: D:\Users\jhlian.m2\repository\com\graphql-java\graphql-java\22.0\graphql-java-22.0.jar(graphql/ExecutionResult.class)
Class file has incorrect version 55.0, 应为 52.0
Please delete the file or ensure that it is located in the correct classpath subdirectory.

@bbakerman
Copy link
Member

Class file has incorrect version 55.0, 应为 52.0

https://mkyong.com/java/list-of-java-class-file-major-version-numbers/

This says the class has version 55.0 (aka JDK 11) and then it says 应为 52.0 which I cant read but I am guessing that you are not running JDK 17 or 11 but in fact JDK 8 since that 52.0 is JDK8

This would be expected - ensure you are running the version you think you are.

Also please dont just put a comment on a random issue. Create a discussion item here https://github.com/graphql-java/graphql-java/discussions

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

5 participants