change: no longer include Position information when marshaling to JSON #364

robmyersrobmyers · 2025-04-11T08:26:32Z

I have a need to marshal the AST to JSON to input into Open Policy Agent, but when I do this for non-trivial schemas the Position information takes a massive amount of memory.
This PR aligns the JSON Marshal rules with the dump rules, which also do not marshal Position.

I have attached a reproducer to demonstrate the issue.

# On master this allocates 30+GB of memory and never finishes.

$ go build -o reproducer reproducer.go && time ./reproducer
Now passing 1262568 bytes to ParseSchema() to demonstrate the benefit of the json marshaling change
Alloc = 4160 MiB	TotalAlloc = 5630 MiB	Sys = 5578 MiB	NumGC = 18
Alloc = 8312 MiB	TotalAlloc = 11174 MiB	Sys = 11120 MiB	NumGC = 19
Alloc = 19392 MiB	TotalAlloc = 22254 MiB	Sys = 22203 MiB	NumGC = 19
Alloc = 16616 MiB	TotalAlloc = 22254 MiB	Sys = 22203 MiB	NumGC = 20
Alloc = 16616 MiB	TotalAlloc = 22254 MiB	Sys = 22203 MiB	NumGC = 20
Alloc = 16616 MiB	TotalAlloc = 22254 MiB	Sys = 22203 MiB	NumGC = 20
Alloc = 33223 MiB	TotalAlloc = 44405 MiB	Sys = 44371 MiB	NumGC = 21
Alloc = 33223 MiB	TotalAlloc = 44405 MiB	Sys = 44371 MiB	NumGC = 21
Alloc = 33223 MiB	TotalAlloc = 44405 MiB	Sys = 44371 MiB	NumGC = 21
Alloc = 33223 MiB	TotalAlloc = 44405 MiB	Sys = 44371 MiB	NumGC = 21
Alloc = 33223 MiB	TotalAlloc = 44405 MiB	Sys = 44371 MiB	NumGC = 21

# With the position JSON annotation change this finishes in well under a second.

$ go build -o reproducer reproducer.go && time ./reproducer
Now passing 1262568 bytes to ParseSchema() to demonstrate the benefit of the json marshaling change
Got 3705551 bytes of JSON

real	0m0.380s
user	0m0.075s
sys	0m0.020s

All tests pass, so I do not think this change breaks any supported use case.

$ go test  ./...
?   	github.com/vektah/gqlparser/v2	[no test files]
ok  	github.com/vektah/gqlparser/v2/ast	0.197s
ok  	github.com/vektah/gqlparser/v2/formatter	0.706s
ok  	github.com/vektah/gqlparser/v2/gqlerror	0.849s
ok  	github.com/vektah/gqlparser/v2/lexer	0.527s
?   	github.com/vektah/gqlparser/v2/main	[no test files]
ok  	github.com/vektah/gqlparser/v2/parser	0.366s
?   	github.com/vektah/gqlparser/v2/parser/testrunner	[no test files]
ok  	github.com/vektah/gqlparser/v2/validator	1.195s
ok  	github.com/vektah/gqlparser/v2/validator/rules	0.994s

coveralls · 2025-04-11T08:28:56Z

coverage: 87.597%. remained the same
when pulling 7d05152 on robmyersrobmyers:no_marshal_position
into bcc3a70 on vektah:master.

… marshaled to JSON. This makes the JSON roundtrip succeed without allocating a ton of memory for JSON fields that will be subsequently pruned. Upstream PR: vektah/gqlparser#364 # Without this change: $ time ./reproducer-pre-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() Alloc = 4159 MiB TotalAlloc = 5625 MiB Sys = 5584 MiB NumGC = 18 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 38765 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 20 Alloc = 33223 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 21 With this change: $ time ./reproducer-post-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() real 0m0.545s user 0m0.201s sys 0m0.028s

… marshaled to JSON. This makes the JSON roundtrip succeed without allocating a ton of memory for JSON fields that will be subsequently pruned. Upstream PR: vektah/gqlparser#364 # Without this change: $ time ./reproducer-pre-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() Alloc = 4159 MiB TotalAlloc = 5625 MiB Sys = 5584 MiB NumGC = 18 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 38765 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 20 Alloc = 33223 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 21 With this change: $ time ./reproducer-post-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() real 0m0.545s user 0m0.201s sys 0m0.028s Signed-off-by: Rob Myers <1243316+robmyersrobmyers@users.noreply.github.com>

… marshaled to JSON. This makes the JSON roundtrip succeed without allocating a ton of memory for JSON fields that will be subsequently pruned. Upstream PR: vektah/gqlparser#364 # Without this change: $ time ./reproducer-pre-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() Alloc = 4159 MiB TotalAlloc = 5625 MiB Sys = 5584 MiB NumGC = 18 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 38765 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 20 Alloc = 33223 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 21 # With this change: $ time ./reproducer-post-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() real 0m0.545s user 0m0.201s sys 0m0.028s Signed-off-by: Rob Myers <1243316+robmyersrobmyers@users.noreply.github.com>

… marshaled to JSON. This makes the JSON roundtrip succeed without allocating a ton of memory for JSON fields that will be subsequently pruned. Upstream PR: vektah/gqlparser#364 # Without this change: $ time ./reproducer-pre-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() Alloc = 4159 MiB TotalAlloc = 5625 MiB Sys = 5584 MiB NumGC = 18 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 38765 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 20 Alloc = 33223 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 21 With this change: $ time ./reproducer-post-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() real 0m0.545s user 0m0.201s sys 0m0.028s

Annotate internal/gqlparser structs not to include Position when marshaled to JSON. This makes the JSON roundtrip succeed without allocating a ton of memory for JSON fields that will be subsequently pruned. Upstream PR: vektah/gqlparser#364 $ time ./reproducer-pre-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() Alloc = 4159 MiB TotalAlloc = 5625 MiB Sys = 5584 MiB NumGC = 18 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 38765 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 20 Alloc = 33223 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 21 $ time ./reproducer-post-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() real 0m0.545s user 0m0.201s sys 0m0.028s Signed-off-by: Rob Myers <1243316+robmyersrobmyers@users.noreply.github.com>

…fields (#7509) Annotate internal/gqlparser structs not to include Position when marshaled to JSON. This makes the JSON roundtrip succeed without allocating a ton of memory for JSON fields that will be subsequently pruned. Upstream PR: vektah/gqlparser#364 $ time ./reproducer-pre-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() Alloc = 4159 MiB TotalAlloc = 5625 MiB Sys = 5584 MiB NumGC = 18 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 8312 MiB TotalAlloc = 11169 MiB Sys = 11125 MiB NumGC = 19 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 16615 MiB TotalAlloc = 22247 MiB Sys = 22209 MiB NumGC = 20 Alloc = 38765 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 20 Alloc = 33223 MiB TotalAlloc = 44397 MiB Sys = 44377 MiB NumGC = 21 $ time ./reproducer-post-fix Now passing 1262568 bytes to builtinGraphQLParseSchema() to reproduce the issue in ast.InterfaceToValue() real 0m0.545s user 0m0.201s sys 0m0.028s Signed-off-by: Rob Myers <1243316+robmyersrobmyers@users.noreply.github.com>

StevenACoffman · 2025-04-17T11:42:47Z

~~Sorry for sitting on this. I would like to merge this, but I maintain this as a volunteer after hours and verifying this particular PR would take more time than I've had. Can you help me?~~

~~I can see how this entirely solves the memory consumption issue, but my concern is that it may break existing behavior since it is not an opt-in or opt-out change.~~

In order to verify that it doesn't break gqlgen, I would need to make a fork (or branch), pin it to your version in the root go.mod and also in the _examples/go.mod, run go mod tidy in both, re-generate (go generate ./... in the root) and commit that and push that up as PR (even in draft) and mention me. That will give me the confidence that it doesn't break anything that a large segment of the ecosystem relies on.

~~Is that a fair ask?~~

Ok, nevermind. I'm just going to carve out some time and do that myself. I feel bad to have made you wait this long already.

StevenACoffman · 2025-04-17T11:56:56Z

Ok, works in gqlgen locally.

change: no longer include Position information when marshaling to JSON

7d05152

robmyersrobmyers mentioned this pull request Apr 11, 2025

Add JSON annotation in internal/gqlparser/ast to Position fields open-policy-agent/opa#7509

Merged

StevenACoffman merged commit a4eb489 into vektah:master Apr 17, 2025
7 checks passed

This was referenced Apr 17, 2025

Update github.com/vektah/gqlparser to v2.5.25 99designs/gqlgen#3669

Merged

Possible to cache graphql.parse() results? open-policy-agent/opa#5377

Closed

robmyersrobmyers deleted the no_marshal_position branch May 6, 2025 08:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

change: no longer include Position information when marshaling to JSON #364

change: no longer include Position information when marshaling to JSON #364

Uh oh!

robmyersrobmyers commented Apr 11, 2025

Uh oh!

coveralls commented Apr 11, 2025

Uh oh!

StevenACoffman commented Apr 17, 2025 •

edited

Loading

Uh oh!

StevenACoffman commented Apr 17, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

change: no longer include Position information when marshaling to JSON #364

change: no longer include Position information when marshaling to JSON #364

Uh oh!

Conversation

robmyersrobmyers commented Apr 11, 2025

Uh oh!

coveralls commented Apr 11, 2025

Uh oh!

StevenACoffman commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StevenACoffman commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StevenACoffman commented Apr 17, 2025 •

edited

Loading

StevenACoffman commented Apr 17, 2025 •

edited

Loading