Skip to content

Commit 56c9aa5

Browse files
robmyersrobmyersrobmyersrobmyers
authored
gqlparser: Add JSON annotation in internal/gqlparser/ast to Position fields (open-policy-agent#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 <[email protected]>
1 parent 9ac8777 commit 56c9aa5

File tree

10 files changed

+36
-20
lines changed

10 files changed

+36
-20
lines changed

‎internal/gqlparser/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ It also will add some linter ignore annotations on the validator rules, since th
1818

1919
The script thus should alleviate around 40-60% of the linter-fixup work required during a version bump.
2020

21+
## JSON Position Marshal script
22+
23+
The `remove-position-marshal.sh` script can be executed from this directory.
24+
This script annotates gqlparser structs to exclude the Position field during JSON marshaling.
25+
The Position field contains a copy of the original schema for each element, potentially causing unexpectedly large memory allocations.
26+
OPA subsequently prunes the Position from the AST, so it makes sense not to generate it in the first place.
27+
2128
## Original README
2229

2330
This is a parser for graphql, written to mirror the graphql-js reference implementation as closely while remaining idiomatic and easy to use.

‎internal/gqlparser/ast/definition.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type Definition struct {
2929
Types []string // union
3030
EnumValues EnumValueList // enum
3131

32-
Position *Position `dump:"-"`
32+
Position *Position `dump:"-" json:"-"`
3333
BuiltIn bool `dump:"-"`
3434
}
3535

@@ -65,7 +65,7 @@ type FieldDefinition struct {
6565
DefaultValue *Value // only for input objects
6666
Type *Type
6767
Directives DirectiveList
68-
Position *Position `dump:"-"`
68+
Position *Position `dump:"-" json:"-"`
6969
}
7070

7171
type ArgumentDefinition struct {
@@ -74,14 +74,14 @@ type ArgumentDefinition struct {
7474
DefaultValue *Value
7575
Type *Type
7676
Directives DirectiveList
77-
Position *Position `dump:"-"`
77+
Position *Position `dump:"-" json:"-"`
7878
}
7979

8080
type EnumValueDefinition struct {
8181
Description string
8282
Name string
8383
Directives DirectiveList
84-
Position *Position `dump:"-"`
84+
Position *Position `dump:"-" json:"-"`
8585
}
8686

8787
type DirectiveDefinition struct {
@@ -90,5 +90,5 @@ type DirectiveDefinition struct {
9090
Arguments ArgumentDefinitionList
9191
Locations []DirectiveLocation
9292
IsRepeatable bool
93-
Position *Position `dump:"-"`
93+
Position *Position `dump:"-" json:"-"`
9494
}

‎internal/gqlparser/ast/directive.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const (
3030
type Directive struct {
3131
Name string
3232
Arguments ArgumentList
33-
Position *Position `dump:"-"`
33+
Position *Position `dump:"-" json:"-"`
3434

3535
// Requires validation
3636
ParentDefinition *Definition

‎internal/gqlparser/ast/document.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package ast
33
type QueryDocument struct {
44
Operations OperationList
55
Fragments FragmentDefinitionList
6-
Position *Position `dump:"-"`
6+
Position *Position `dump:"-" json:"-"`
77
}
88

99
type SchemaDocument struct {
@@ -12,7 +12,7 @@ type SchemaDocument struct {
1212
Directives DirectiveDefinitionList
1313
Definitions DefinitionList
1414
Extensions DefinitionList
15-
Position *Position `dump:"-"`
15+
Position *Position `dump:"-" json:"-"`
1616
}
1717

1818
func (d *SchemaDocument) Merge(other *SchemaDocument) {
@@ -69,11 +69,11 @@ type SchemaDefinition struct {
6969
Description string
7070
Directives DirectiveList
7171
OperationTypes OperationTypeDefinitionList
72-
Position *Position `dump:"-"`
72+
Position *Position `dump:"-" json:"-"`
7373
}
7474

7575
type OperationTypeDefinition struct {
7676
Operation Operation
7777
Type string
78-
Position *Position `dump:"-"`
78+
Position *Position `dump:"-" json:"-"`
7979
}

‎internal/gqlparser/ast/fragment.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ type FragmentSpread struct {
88
ObjectDefinition *Definition
99
Definition *FragmentDefinition
1010

11-
Position *Position `dump:"-"`
11+
Position *Position `dump:"-" json:"-"`
1212
}
1313

1414
type InlineFragment struct {
@@ -19,7 +19,7 @@ type InlineFragment struct {
1919
// Require validation
2020
ObjectDefinition *Definition
2121

22-
Position *Position `dump:"-"`
22+
Position *Position `dump:"-" json:"-"`
2323
}
2424

2525
type FragmentDefinition struct {
@@ -34,5 +34,5 @@ type FragmentDefinition struct {
3434
// Require validation
3535
Definition *Definition
3636

37-
Position *Position `dump:"-"`
37+
Position *Position `dump:"-" json:"-"`
3838
}

‎internal/gqlparser/ast/operation.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ type OperationDefinition struct {
1414
VariableDefinitions VariableDefinitionList
1515
Directives DirectiveList
1616
SelectionSet SelectionSet
17-
Position *Position `dump:"-"`
17+
Position *Position `dump:"-" json:"-"`
1818
}
1919

2020
type VariableDefinition struct {
2121
Variable string
2222
Type *Type
2323
DefaultValue *Value
2424
Directives DirectiveList
25-
Position *Position `dump:"-"`
25+
Position *Position `dump:"-" json:"-"`
2626

2727
// Requires validation
2828
Definition *Definition

‎internal/gqlparser/ast/selection.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type Field struct {
2121
Arguments ArgumentList
2222
Directives DirectiveList
2323
SelectionSet SelectionSet
24-
Position *Position `dump:"-"`
24+
Position *Position `dump:"-" json:"-"`
2525

2626
// Require validation
2727
Definition *FieldDefinition
@@ -31,7 +31,7 @@ type Field struct {
3131
type Argument struct {
3232
Name string
3333
Value *Value
34-
Position *Position `dump:"-"`
34+
Position *Position `dump:"-" json:"-"`
3535
}
3636

3737
func (s *Field) ArgumentMap(vars map[string]interface{}) map[string]interface{} {

‎internal/gqlparser/ast/type.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type Type struct {
2020
NamedType string
2121
Elem *Type
2222
NonNull bool
23-
Position *Position `dump:"-"`
23+
Position *Position `dump:"-" json:"-"`
2424
}
2525

2626
func (t *Type) Name() string {

‎internal/gqlparser/ast/value.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type Value struct {
2525
Raw string
2626
Children ChildValueList
2727
Kind ValueKind
28-
Position *Position `dump:"-"`
28+
Position *Position `dump:"-" json:"-"`
2929

3030
// Require validation
3131
Definition *Definition
@@ -36,7 +36,7 @@ type Value struct {
3636
type ChildValue struct {
3737
Name string
3838
Value *Value
39-
Position *Position `dump:"-"`
39+
Position *Position `dump:"-" json:"-"`
4040
}
4141

4242
func (v *Value) Value(vars map[string]interface{}) (interface{}, error) {

‎internal/gqlparser/remove-position-marshal.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/usr/bin/env bash
2+
set -e
3+
4+
# Relies on perl to do in-place regex search-and-replace for the Position annotations
5+
# Relies on find to recursively enumerate all the Go files.
6+
7+
# Add Position json annotation
8+
# Position information is pruned by pruneIrrelevantGraphQLASTNodes() later anyway
9+
for f in $(find . -name "*.go"); do perl -pi -e 's/\*Position \`dump:"-"\`/\*Position \`dump:"-" json:"-"\`/g' $f; done

0 commit comments

Comments
 (0)