gqlgen/parser

Masahiro Wakame

@vvakame

Writing as usual would do as follows.
This is a confusing bug that values are ghostly eaten.

    for _, child := range w.Document.Operations {
        w.validatedFragmentSpreads = make(map[string]bool)
        w.walkOperation(&child)
    }

Masahiro Wakame

@vvakame

I will return home from now so check here again later!

Adam Scarr

@vektah

right, so because

type QueryDocument struct {
    Operations []OperationDefinition
}

operations is all allocated contiguously in memory (its very efficient to loop over due to cache coherency, but adding elements can be costly)

Operations: [OperationDefinition(Operation: "query", Name: "", ...)][OperationDefinition(Operation: "query", Name: "", ...)][OperationDefinition(Operation: "query", Name: "", ...)]

the problem is a for loop copies it when traversing:

for i, def := range document.Operation {
    // &def is a pointer to a copy
   // &document.Operation[i] is a pointer to the element of the array
}

wheras if

type QueryDocument struct {
    Operations []*OperationDefinition // note pointer
}

you end up with an array of pointers, referencing objects randomly allocated all over the place

op 3: [OperationDefinition(Operation: "query", Name: "", ...)]
[ptr to 1][ptr to 2][ptr to 3]
op 1: [OperationDefinition(Operation: "query", Name: "", ...)]
op 2: [OperationDefinition(Operation: "query", Name: "", ...)]

when you loop over the pointers, go still copies them, but its a copy of a pointer so it doesnt matter so much, the cost you pay is now your reading things from random locations in memory as you loop, which is much slower if you do it a lot (for operation it probably doesnt matter, you expect a few items max

feel free to make []*things wherever you need, we can always wind them back to []things if they are expected to be lager

pretty sure contiguous arrays are also less pressure on the gc, not having to clean up lots of small references. perf without benchmarks is dumb though, so do what makes sense

Masahiro Wakame

@vvakame

okay, I'll try it later.

Masahiro Wakame

@vvakame

I started going forward!

Adam Scarr

@vektah

and I broke all your code?

vektah/gqlparser#39 makes all the collections []*Thing

Masahiro Wakame

@vvakame

yeah I found it and merge it in 1 min :smiley_cat:

Masahiro Wakame

@vvakame

@vektah I have a question.
https://github.com/vektah/gqlparser/blob/master/spec/validation/deviations.yml#L13-L26
Why these tests are marked skip: true ?

Adam Scarr

@vektah

inherited from the upstream library

we should try and get them implemented

https://github.com/graph-gophers/graphql-go/blob/master/internal/validation/testdata/export.js#L76-L111 ftr they have lots of skips

plus the other ones https://github.com/graph-gophers/graphql-go/blob/master/internal/validation/testdata/export.js#L48-L66

Masahiro Wakame

@vvakame

make sense :thinking_face:

Masahiro Wakame

@vvakame

From tomorrow I will return to another job that is stacked. :wink:
Please let me know if you have anything you would like me to help with.
I can not do much as I go to SanFransisco next week.

Adam Scarr

@vektah

oh wow you finished it!

amazing :heart_eyes:

Adam Scarr

@vektah

@vvakame I've sent you an collaborator invite for gqlparser

I'm tracking stuff for "1.0" on https://github.com/vektah/gqlgen/projects/1, if you see something you want to work on feel free

just drag it into "in progress" so someone else doesn't start working on it

I think Mat will start to break down that directive card some more once he gets his head around how it will all hoook in

Masahiro Wakame

@vvakame

wow, thx. I accepted invitation.

Adam Scarr

@vektah

thanks for all the hard work :D

Masahiro Wakame

@vvakame

If you did not make your code at first, I would not be eager to GraphQL. thanks for your hard work too :sparkles:

Tamal Saha

@tamalsaha

Hello, just trying to learn about GraphQL in Go.

Have you looked into using https://github.com/graphql-go/graphql/tree/master/language parser instead of writing your own?

Adam Scarr

@vektah

Yeah, I looked at a bunch

the parser and lexer are actually really easy

the validation is hard

Adam Scarr

@vektah

That library also doesnt use any of the schema IDL, so while there is code there its not being used for anything

also line count comparisons are interesting,

   110 ./benchutil/list_schema.go
   144 ./benchutil/wide_schema.go
  1311 ./definition.go
   148 ./directives.go
    74 ./examples/context/main.go
   141 ./examples/custom-scalar-type/main.go
    41 ./examples/hello-world/main.go
   114 ./examples/http/main.go
   138 ./examples/httpdynamic/main.go
    75 ./examples/modify-context/main.go
    24 ./examples/star-wars/main.go
   225 ./examples/todo/main.go
   872 ./executor.go
    78 ./gqlerrors/error.go
    62 ./gqlerrors/formatted.go
    39 ./gqlerrors/located.go
    30 ./gqlerrors/sortutil.go
    69 ./gqlerrors/syntax.go
    63 ./graphql.go
   763 ./introspection.go
    29 ./language/ast/arguments.go
   235 ./language/ast/definitions.go
    33 ./language/ast/directives.go
    31 ./language/ast/document.go
    22 ./language/ast/location.go
    28 ./language/ast/name.go
    45 ./language/ast/node.go
   134 ./language/ast/selections.go
   103 ./language/ast/types.go
   529 ./language/ast/type_definitions.go
   302 ./language/ast/values.go
    59 ./language/kinds/kinds.go
   682 ./language/lexer/lexer.go
    35 ./language/location/location.go
  1607 ./language/parser/parser.go
   822 ./language/printer/printer.go
    20 ./language/source/source.go
    11 ./language/typeInfo/type_info.go
   736 ./language/visitor/visitor.go
    51 ./located.go
  1892 ./rules.go
   706 ./rules_overlapping_fields_can_be_merged.go
   433 ./scalars.go
   538 ./schema.go
    99 ./testutil/introspection_query.go
   622 ./testutil/rules_test_harness.go
   468 ./testutil/testutil.go
    16 ./types.go
   276 ./type_info.go
   192 ./util.go
   287 ./validator.go
   421 ./values.go
 15985 total

   138 ./ast/collections.go
    92 ./ast/definition.go
    38 ./ast/directive.go
    65 ./ast/document.go
   159 ./ast/dumper.go
    38 ./ast/fragment.go
    29 ./ast/operation.go
    35 ./ast/selection.go
    14 ./ast/source.go
    68 ./ast/type.go
   115 ./ast/value.go
    88 ./gqlerror/error.go
    35 ./gqlparser.go
    58 ./lexer/blockstring.go
   510 ./lexer/lexer.go
   148 ./lexer/token.go
   112 ./parser/parser.go
   334 ./parser/query.go
   503 ./parser/schema.go
   138 ./parser/testrunner/runner.go
    55 ./validator/error.go
    30 ./validator/inliner/inliner.go
    39 ./validator/messaging.go
     5 ./validator/prelude.go
    86 ./validator/rules/fields_on_correct_type.go
    39 ./validator/rules/fragments_on_composite_types.go
    57 ./validator/rules/known_argument_names.go
    31 ./validator/rules/known_directives.go
    19 ./validator/rules/known_fragment_names.go
    61 ./validator/rules/known_type_names.go
    19 ./validator/rules/lone_anonymous_operation.go
    93 ./validator/rules/no_fragment_cycles.go
    28 ./validator/rules/no_undefined_variables.go
    30 ./validator/rules/no_unused_fragments.go
    30 ./validator/rules/no_unused_variables.go
   553 ./validator/rules/overlapping_fields_can_be_merged.go
    68 ./validator/rules/possible_fragment_spreads.go
    63 ./validator/rules/provided_required_arguments.go
    36 ./validator/rules/scalar_leafs.go
    30 ./validator/rules/single_field_subscriptions.go
    33 ./validator/rules/unique_argument_names.go
    24 ./validator/rules/unique_directives_per_location.go
    22 ./validator/rules/unique_fragment_names.go
    27 ./validator/rules/unique_input_field_names.go
    22 ./validator/rules/unique_operation_names.go
    23 ./validator/rules/unique_variable_names.go
   130 ./validator/rules/values_of_correct_type.go
    28 ./validator/rules/variables_are_input_types.go
    36 ./validator/rules/variables_in_allowed_position.go
   199 ./validator/schema.go
    69 ./validator/suggestionList.go
    44 ./validator/validator.go
   286 ./validator/walk.go
  5032 total

Tamal Saha

@tamalsaha

Thanks @vektah . With every release of GraphQL does the language change?

Adam Scarr

@vektah

yeah, gqlparser is importing the testsuite from graphql/graphql-js to help keep up to date

there was a lot in the last spec release https://github.com/facebook/graphql/releases

Tamal Saha

@tamalsaha

This is generally the worry with hand written parsers. Writing once is easy. Keeping up with the change gets boring.

Adam Scarr

@vektah

its close enough to the js implementation that it should be pretty easy

the old parser (https://github.com/graph-gophers/graphql-go/tree/master/internal) had fallen behind a fair bit

Tamal Saha

@tamalsaha

I don't want to be a downer. But say some genius at FB thinks he can rewrite the parser in some better way and does that is some future release. Now Go impl is not close.

I have seen this sort of thing.

Adam Scarr

@vektah

sure, I'm not too worried

the implementation is pretty clean

Tamal Saha

@tamalsaha

That's great.

Adam Scarr

@vektah

and like I said, most of the hard stuff is in validation

parser code is really pretty boring, straight forward code

Tamal Saha

@tamalsaha

ok.

Adam Scarr

@vektah

this thing https://github.com/graphql-go/graphql/blob/master/rules_overlapping_fields_can_be_merged.go

Tamal Saha

@tamalsaha

Are you familiar with GRPC ? We are currently using that . I am trying to see if GraphQL can help.

Where communities thrive

People

Repo info

Activity