thunder
thunder copied to clipboard
go federation: add fix for infinite recursion
This PR adds a fix for an infinite recursion bug in flattening code.
The bug is rooted in the mergeSameAlias function, where we combine selections with the same alias's subselections. However, because we do not deepcopy the selections before modifying them, and because our fragments have pointers to the same selections, this results in modifying the underlying subselections.
Performance: Since this approach uses deepcopying, we can reasonably expect this to impact performance. Here are the results from benchmarking:
Current
BenchmarkNormalize2-8 5182 193595 ns/op 101487 B/op 2131 allocs/op
DeepCopyInMerge:
BenchmarkNormalize2-8 613 1665200 ns/op 810135 B/op 22725 allocs/op
DeepCopyInFlatten:
BenchmarkNormalize2-8 619 1845102 ns/op 880146 B/op 24415 allocs/op
As a result of this change, we see a roughly 760% runtime increase and 1000% memory utilization increase.
The DeepCopy in Merge refers to an alternative solution where we add the deepcopy to the mergeSameAlias function, here:
copy := *selection
selection = ©
newSelections = append(newSelections, selection)
last = selection
I changed the deep copy to only copy the first layer of selections and fragments, the benchmark is as below:
BenchmarkNormalize2-16 5040 248245 ns/op 105241 B/op 2232 allocs/op
The root cause is last.SelectionSet.Selections = append(last.SelectionSet.Selections, s)
sometimes create a new slice, sometimes not, when it does not create the new slice, we will run into the bug.