grooviter / gql

Groovy GraphQL library
http://grooviter.github.io/gql/
Apache License 2.0
49 stars 6 forks source link

Transfer link to GraphQL #26

Closed KraftNSK closed 4 years ago

KraftNSK commented 4 years ago

To improve performance, I suggest for the DSL.execute(...) method to add the ability to transfer a link to an instance of the GraphQL

mariogarcia commented 4 years ago

Hi @KraftNSK:

Could you explain a little bit more ? What do you mean by transferring a link to an instance of the GraphQL ?

KraftNSK commented 4 years ago

Hi! The current signature of the execute does not allow you to transfer a link to the previously instantiated graphql (on the same schema). Judging by the code, each call executes leads to the creation of a new graphql. If there are a lot of requests - memory and CPU overhead will be noticeable + delays

mariogarcia commented 4 years ago

Ah! I think I got it, correct me if I'm wrong... if you executed:

DSL.execute(schema, query)

That creates a new GraphQL instance everytime with the consequent overhead. Instead it would be better if we could pass the GraphQL instance to avoid creating it every time, something like:

def graphQL = ...

result1 = DSL.execute(graphQL, query1)
resul2 = DSL.execute(graphQL, query2)

Is that what you were looking for ?

BTW I agree there should be such method. In fact maybe there's room for another DSL method, I imagine something like:

def executor = DSL.createExecutor(schema)

def result1 = executor.execute(query, variables)
def result2 = executor.execute(query, variables)

These examples are supposed to be using a reusable instance to do as many queries as you want. Maybe the naming could vary, but it shows the idea.

What do you think ?

KraftNSK commented 4 years ago

Yes, you understood me correctly! In my project, I did just that. As a result, with a large number of requests, a significant amount of resources is saved.

mariogarcia commented 4 years ago

I'm thinking about exposing directly the GraphQL.Builder instance from DSL class and getting rid of execute methods. Main reasons are:

However when bootstraping the GraphQL engine, there're a couple of methods that I think may be of use:

    GraphQLSchema schema = null
    Instrumentation instrumentation = null

    // create GraphQL instance from an existing schema
    GraphQL.Builder exec1 = DSL.executor(schema)

    // create GraphQL instance from an existing schema, and add instrumentations
    GraphQL.Builder exec2 = DSL.executor(schema, instrumentation)

    // Creating the schema and GraphQL instance at the same time. Nice for micro-services
    GraphQL.Builder exec3 = DSL.executor {
      type("") {
        field("greeting", GraphQLString)
      }
    }

    // Similar like the prevous one but enabling instrumentation directly
    GraphQL.Builder exec4 = DSL.executor(instrumentation) {
      type("") {
        field("greeting", GraphQLString)
      }
    }

The plan is to add some of these methods in version 0.4.0 and maybe get rid of execute(...) methods in a future 1.0.0 version.

KraftNSK commented 4 years ago

Hello! When is it planned to release version 0.4.0?

mariogarcia commented 4 years ago

I'm thinking it could be release in two weeks from now. I'm updating the milestone expected due date. Maybe is sooner, but two weeks is a realistic estimate.

mariogarcia commented 4 years ago

update: I'm falling behind but it's on my radar