Skip to content

Commit

Permalink
Fix world's slowest stack overflow (#123)
Browse files Browse the repository at this point in the history
* Fix world's slowest stack overflow
When dealing with CRDsonnet libraries in our (Grafana Labs) repositories, the language server would lock up:

- I thought it had something to do with CRDsonnet but I was wrong.
- This happened because it was trying to find a field in the wrong nodestack repeatedly (see image)
- This PR fixes the issue.
- Obviously, the language server can't find fields that come from processing jsonschema, but at least it doesn't take up 4 full CPUs (it reached a full 1% CPU usage while testing :) )

* Remove unneeded boolean
All tests are still passing. `$` can be resolved across multiple files, so I guess the language server is just better now and doesn't need that safety
If this causes a bug, we should add a new test
  • Loading branch information
julienduchesne authored Aug 29, 2023
1 parent 9708978 commit a23b945
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 8 deletions.
20 changes: 12 additions & 8 deletions pkg/ast/processing/find_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm
var foundDesugaredObjects []*ast.DesugaredObject
// First element will be super, self, or var name
start, indexList := indexList[0], indexList[1:]
sameFileOnly := false
switch {
case start == "super":
// Find the LHS desugared object of a binary node
Expand All @@ -36,7 +35,6 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm
case start == "std":
return nil, fmt.Errorf("cannot get definition of std lib")
case start == "$":
sameFileOnly = true
foundDesugaredObjects = FindTopLevelObjects(nodestack.NewNodeStack(stack.From), vm)
case strings.Contains(start, "."):
foundDesugaredObjects = FindTopLevelObjectsInFile(vm, start, "")
Expand Down Expand Up @@ -66,6 +64,7 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm
case *ast.Import:
filename := bodyNode.File.Value
foundDesugaredObjects = FindTopLevelObjectsInFile(vm, filename, "")

case *ast.Index, *ast.Apply:
tempStack := nodestack.NewNodeStack(bodyNode)
indexList = append(tempStack.BuildIndexList(), indexList...)
Expand All @@ -80,10 +79,10 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm
}
}

return extractObjectRangesFromDesugaredObjs(stack, vm, foundDesugaredObjects, sameFileOnly, indexList, partialMatchFields)
return extractObjectRangesFromDesugaredObjs(vm, foundDesugaredObjects, indexList, partialMatchFields)
}

func extractObjectRangesFromDesugaredObjs(stack *nodestack.NodeStack, vm *jsonnet.VM, desugaredObjs []*ast.DesugaredObject, sameFileOnly bool, indexList []string, partialMatchFields bool) ([]ObjectRange, error) {
func extractObjectRangesFromDesugaredObjs(vm *jsonnet.VM, desugaredObjs []*ast.DesugaredObject, indexList []string, partialMatchFields bool) ([]ObjectRange, error) {
var ranges []ObjectRange
for len(indexList) > 0 {
index := indexList[0]
Expand Down Expand Up @@ -135,10 +134,15 @@ func extractObjectRangesFromDesugaredObjs(stack *nodestack.NodeStack, vm *jsonne
case *ast.DesugaredObject:
desugaredObjs = append(desugaredObjs, fieldNode)
case *ast.Index:
additionalIndexList := append(nodestack.NewNodeStack(fieldNode).BuildIndexList(), indexList...)
result, err := FindRangesFromIndexList(stack, additionalIndexList, vm, partialMatchFields)
if len(result) > 0 {
if !sameFileOnly || result[0].Filename == stack.From.Loc().FileName {
// if we're trying to find the a definition which is an index,
// we need to find it from itself, meaning that we need to create a stack
// from the index's target and search from there
rootNode, _, _ := vm.ImportAST("", fieldNode.LocRange.FileName)
stack, _ := FindNodeByPosition(rootNode, fieldNode.Target.Loc().Begin)
if stack != nil {
additionalIndexList := append(nodestack.NewNodeStack(fieldNode).BuildIndexList(), indexList...)
result, _ := FindRangesFromIndexList(stack, additionalIndexList, vm, partialMatchFields)
if len(result) > 0 {
return result, err
}
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/server/definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,22 @@ var definitionTestCases = []definitionTestCase{
},
}},
},
{
name: "test fix infinite recursion",
filename: "./testdata/goto-infinite-recursion-bug-1.jsonnet",
position: protocol.Position{Line: 2, Character: 26},
results: []definitionResult{{
targetFilename: "testdata/goto-infinite-recursion-bug-3.libsonnet",
targetRange: protocol.Range{
Start: protocol.Position{Line: 5, Character: 10},
End: protocol.Position{Line: 5, Character: 36},
},
targetSelectionRange: protocol.Range{
Start: protocol.Position{Line: 5, Character: 10},
End: protocol.Position{Line: 5, Character: 22},
},
}},
},
}

func TestDefinition(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/server/testdata/goto-infinite-recursion-bug-1.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
local drone = import './goto-infinite-recursion-bug-2.libsonnet';
{
steps: drone.step.withCommands([
'blabla',
]),
}
15 changes: 15 additions & 0 deletions pkg/server/testdata/goto-infinite-recursion-bug-2.libsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
local drone = import './goto-infinite-recursion-bug-3.libsonnet';
{
pipeline:
drone.pipeline.docker {
new(name):
super.new(name)
+ super.clone.withRetries(3)
+ super.clone.withDepth(10000)
+ super.platform.withArch('amd64')
+ super.withImagePullSecrets(['dockerconfigjson']),
},

step:
drone.pipeline.docker.step,
}
13 changes: 13 additions & 0 deletions pkg/server/testdata/goto-infinite-recursion-bug-3.libsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
local lib =
{
pipeline: {
docker: {
step: {
withCommands(commands): {},
},
},
},
};


lib

0 comments on commit a23b945

Please sign in to comment.