-
Notifications
You must be signed in to change notification settings - Fork 842
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
General Performance #119
Comments
Hi @Mleonard87 Thanks for taking time to look into the performance of I'm glad that someone is taking up the challenge to figure out weak points in the library, it helps to steer the direction of the development. The code for both Edit: Probably it would help to state the version of Regarding areas of improvements, I can offer some notes that I already have that would help with the effort in improving and optimising the performance.
Both improvements to the Again, we appreciate that work you put into this, we welcome your contribution very much! Cheers |
Yeah, absolutely, I can run these tests again on your branch later when I get home. I'll also clarify my Also, has there been any discussion on caching the results of the validate/parse? I think that in a lot of applications the same query my be executed often. For example, in a Todo app the main page might fetch a list of all the Todos and the graphql query itself would be the same each time even if the results are different. Have you seen this in any other graphql implementations? |
There is a graphql lib, which is based on libgraphqlparser, https://github.com/tallstreet/graphql, probably not active anymore. One concern I have is using it in sandboxed cloud services like google app engine, which (used to) restricts the use of |
Here are benchmarks against the same code for different versions of I ran each test 5 times and the full results can be found here but I've included just the best for each below. They don't vary enough between tests to worry about. Things to note:
Overall your new branch is showing incredible performance - totally was't expecting this. Amazing! Versions
Specs
Benchmarksgraphql-js 0.4.3
graphql-go master
graphql-js 0.4.18
graphql-go sogko/0.4.18
|
@bbuck That is an interesting insight, we have to keep this in mind (to use or not to use cgo) and figure out how to go about doing this. |
Hi @Mleonard87 Thanks for running more benchmark tests for the different configurations, really appreciate the time you put into this 👍 Woah, those results seems really promising, I'm quite surprised myself lol. In the future, we could possibly have a separate repo within /cc @chris-ramon |
A benchmark repo is not a bad idea. Probably need something more complex than my very trivial hello world test case. I had wondered myself about other libraries. I might them a go when I get some spare time. If these times can be maintained once PR #117 is complete then I think this can be a blazingly fast library. |
I have checked the performance too and found that the current implementation of the Lexer produces a lot of garbage and slows everything by orders of magnitudes. For details and a possible fix see here: #137 |
@Matthias247 Nice tip got to say its best to use bytes.Buffer in golang then to use strings since you can pool and reuse them. We use them a lot and even fast frameworks like https://github.com/valyala/fasthttp and https://github.com/labstack/echo use them to get the highest speed. Anyway I ran the benchmark on my machine against our implementation of graphql, graphql-go master
playlyfe/go-graphql master
And BTW shouldn't these be in benchmark tests in the library and not like this then we could figure out allocs/sec and ops/s also. package main
import (
"encoding/json"
"fmt"
"net/http"
"github.com/playlyfe/go-graphql"
)
func main() {
schema := `
type RootQueryType {
hello: String
}
`
resolvers := map[string]interface{}{}
resolvers["RootQueryType/hello"] = func(params *graphql.ResolveParams) (interface{}, error) {
return "world", nil
}
context := map[string]interface{}{}
variables := map[string]interface{}{}
executor, err := graphql.NewExecutor(schema, "RootQueryType", "", resolvers)
if err != nil {
panic(err)
}
http.HandleFunc("/graphql", func(w http.ResponseWriter, r *http.Request) {
result, err := executor.Execute(context, r.URL.Query()["query"][0], variables, "")
if err != nil {
panic(err)
}
json.NewEncoder(w).Encode(result)
})
fmt.Println("Benchmark app listening on port 3003!")
http.ListenAndServe(":3003", nil)
} |
I did a benchmark without the http overhead and using the go test tool and this is what i get,
Here's the code, package graphql_test
import (
"testing"
"github.com/graphql-go/graphql"
pgql "github.com/playlyfe/go-graphql"
)
var schema, _ = graphql.NewSchema(
graphql.SchemaConfig{
Query: graphql.NewObject(
graphql.ObjectConfig{
Name: "RootQueryType",
Fields: graphql.Fields{
"hello": &graphql.Field{
Type: graphql.String,
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
return "world", nil
},
},
},
}),
},
)
func BenchmarkGoGraphQLMaster(b *testing.B) {
for i := 0; i < b.N; i++ {
graphql.Do(graphql.Params{
Schema: schema,
RequestString: "{hello}",
})
}
}
var schema2 = `
type RootQueryType {
hello: String
}
`
var resolvers = map[string]interface{}{
"RootQueryType/hello": func(params *pgql.ResolveParams) (interface{}, error) {
return "world", nil
},
}
var executor, _ = pgql.NewExecutor(schema2, "RootQueryType", "", resolvers)
func BenchmarkPlaylyfeGraphQLMaster(b *testing.B) {
for i := 0; i < b.N; i++ {
context := map[string]interface{}{}
variables := map[string]interface{}{}
executor.Execute(context, "{hello}", variables, "")
}
} |
thanks for @pyros2097, I add graph-gophers/graphql-go in benchnark lint. See the repo golang-graphql-benchmark result:
code: package graphql_test
import (
"context"
"testing"
ggql "github.com/graph-gophers/graphql-go"
"github.com/graphql-go/graphql"
pgql "github.com/playlyfe/go-graphql"
)
var schema, _ = graphql.NewSchema(
graphql.SchemaConfig{
Query: graphql.NewObject(
graphql.ObjectConfig{
Name: "RootQueryType",
Fields: graphql.Fields{
"hello": &graphql.Field{
Type: graphql.String,
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
return "world", nil
},
},
},
}),
},
)
func BenchmarkGoGraphQLMaster(b *testing.B) {
for i := 0; i < b.N; i++ {
graphql.Do(graphql.Params{
Schema: schema,
RequestString: "{hello}",
})
}
}
var schema2 = `
type RootQueryType {
hello: String
}
`
var resolvers = map[string]interface{}{
"RootQueryType/hello": func(params *pgql.ResolveParams) (interface{}, error) {
return "world", nil
},
}
var executor, _ = pgql.NewExecutor(schema2, "RootQueryType", "", resolvers)
func BenchmarkPlaylyfeGraphQLMaster(b *testing.B) {
for i := 0; i < b.N; i++ {
context := map[string]interface{}{}
variables := map[string]interface{}{}
executor.Execute(context, "{hello}", variables, "")
}
}
type helloWorldResolver1 struct{}
func (r *helloWorldResolver1) Hello() string {
return "world"
}
var schema3 = ggql.MustParseSchema(`
schema {
query: Query
}
type Query {
hello: String!
}
`, &helloWorldResolver1{})
func BenchmarkGophersGraphQLMaster(b *testing.B) {
for i := 0; i < b.N; i++ {
ctx := context.Background()
variables := map[string]interface{}{}
schema3.Exec(ctx, "{hello}", "", variables)
}
} |
Hi, so we are using this library at work and were looking at a certain optimisation.
We've increased maximum number of resolvers per request to 50 as of now. Quick question though, was there a specific reason why Thank you again for creating an amazing library for people to work with. |
Hi @salman-bhai, we don't actually set a Perhaps are you referring to the limitation of a different library: |
According to #106 performance has not been a key concern at the moment over functionality, with which I completely agree. However, out of interest I started running some simple and contrived load testing using wrk. I compared an absolutely barebones graphql setup in graphql-go and express-graphql (code in gists linked to below) and here are some results for a test with 12 threads, 400 connections over a 30 second period.
Notable points are:
Given the claim of no real optimisations so far I think this is an excellent starting point, especially given the significantly lower failure rate. However, I think for such a trivial test case the timing should be must closer.
I'm pretty new to go but I'm looking to further my skills. I'm going to try and tackle some of the other open issues first but I would like to come back to this and help where I can. Perhaps in the meantime we could start a discussion on how to improve and discover some areas of code that could be investigated.
I have also attached a flame graph at the bottom that was sampled for 15 seconds during the middle a 30 second load test. This indicates that most of the time spent in graphql-go is spent inside
graphql.ValidateDocument()
and specificallyvistor.Visit()
Query Used
express-graphql
graphql-go
Code
graphql-go
express-graphql
Flame Graph
The text was updated successfully, but these errors were encountered: