Skip to content

Commit

Permalink
indexer/walker: Avoid running jobs where not needed (#1006)
Browse files Browse the repository at this point in the history 
* indexer cleanup: Replace Defer with DependsOn

* Introduce StateIgnore & plumb context through

* state: Avoid deduplicating jobs on enqueuing

* indexer: use IgnoreState

* ci: bump benchmarks total timeout

* update benchmarks thresholds

* indexer: run ObtainSchema even if scheduling of an upstream job fails
  • Loading branch information
@radeksimko
radeksimko authored Aug 10, 2022
1 parent aa15239 commit a12c9af
Show file tree
Hide file tree
Showing 14 changed files with 364 additions and 177 deletions.
20 changes: 10 additions & 10 deletions .github/gobenchdata-checks.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ checks:
benchmarks: [BenchmarkInitializeFolder_basic/local-single-module-no-provider]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 25
max: 55
min: 3
max: 20
- package: ./internal/langserver/handlers
name: local-single-submodule-no-provider
benchmarks: [BenchmarkInitializeFolder_basic/local-single-submodule-no-provider]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 140
max: 310
min: 100
max: 250
- package: ./internal/langserver/handlers
name: local-single-module-random
benchmarks: [BenchmarkInitializeFolder_basic/local-single-module-random]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 140
min: 100
max: 300
- package: ./internal/langserver/handlers
name: local-single-module-aws
Expand Down Expand Up @@ -47,35 +47,35 @@ checks:
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 1400
max: 2200
max: 5000
- package: ./internal/langserver/handlers
name: google-project
benchmarks: [BenchmarkInitializeFolder_basic/google-project]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 1570
max: 2450
max: 9000
- package: ./internal/langserver/handlers
name: google-network
benchmarks: [BenchmarkInitializeFolder_basic/google-network]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 1430
max: 2700
max: 15000
- package: ./internal/langserver/handlers
name: google-gke
benchmarks: [BenchmarkInitializeFolder_basic/google-gke]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 1500
max: 5100
max: 20000
- package: ./internal/langserver/handlers
name: k8s-metrics-server
benchmarks: [BenchmarkInitializeFolder_basic/k8s-metrics-server]
diff: current.NsPerOp / 1000000 # ms
thresholds:
min: 1000
max: 3200
max: 4000
- package: ./internal/langserver/handlers
name: k8s-dashboard
benchmarks: [BenchmarkInitializeFolder_basic/k8s-dashboard]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/benchmarks.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
-bench=InitializeFolder_basic \
-run=^# \
-benchtime=60s \
-timeout=30m | tee ${{ runner.temp }}/benchmarks.txt
-timeout=60m | tee ${{ runner.temp }}/benchmarks.txt
-
name: Evaluate benchmarks
id: bench-eval
Expand Down
36 changes: 21 additions & 15 deletions internal/indexer/document_change.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro
parseId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.ParseModuleConfiguration(idx.fs, idx.modStore, modHandle.Path())
return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path())
},
Type: op.OpTypeParseModuleConfiguration.String(),
Type: op.OpTypeParseModuleConfiguration.String(),
IgnoreState: true,
})
if err != nil {
return ids, err
}
ids = append(ids, parseId)

modIds, err := idx.decodeModule(modHandle, job.IDs{parseId})
modIds, err := idx.decodeModule(modHandle, job.IDs{parseId}, true)
if err != nil {
return ids, err
}
Expand All @@ -33,9 +34,10 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro
parseVarsId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.ParseVariables(idx.fs, idx.modStore, modHandle.Path())
return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path())
},
Type: op.OpTypeParseVariables.String(),
Type: op.OpTypeParseVariables.String(),
IgnoreState: true,
})
if err != nil {
return ids, err
Expand All @@ -47,8 +49,9 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro
Func: func(ctx context.Context) error {
return module.DecodeVarsReferences(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeDecodeVarsReferences.String(),
DependsOn: job.IDs{parseVarsId},
Type: op.OpTypeDecodeVarsReferences.String(),
DependsOn: job.IDs{parseVarsId},
IgnoreState: true,
})
if err != nil {
return ids, err
Expand All @@ -58,16 +61,17 @@ func (idx *Indexer) DocumentChanged(modHandle document.DirHandle) (job.IDs, erro
return ids, nil
}

func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs) (job.IDs, error) {
func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs, ignoreState bool) (job.IDs, error) {
ids := make(job.IDs, 0)

metaId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.LoadModuleMetadata(idx.modStore, modHandle.Path())
return module.LoadModuleMetadata(ctx, idx.modStore, modHandle.Path())
},
Type: op.OpTypeLoadModuleMetadata.String(),
DependsOn: dependsOn,
Type: op.OpTypeLoadModuleMetadata.String(),
DependsOn: dependsOn,
IgnoreState: ignoreState,
})
if err != nil {
return ids, err
Expand All @@ -79,8 +83,9 @@ func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs
Func: func(ctx context.Context) error {
return module.DecodeReferenceTargets(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeDecodeReferenceTargets.String(),
DependsOn: job.IDs{metaId},
Type: op.OpTypeDecodeReferenceTargets.String(),
DependsOn: job.IDs{metaId},
IgnoreState: ignoreState,
})
if err != nil {
return ids, err
Expand All @@ -92,8 +97,9 @@ func (idx *Indexer) decodeModule(modHandle document.DirHandle, dependsOn job.IDs
Func: func(ctx context.Context) error {
return module.DecodeReferenceOrigins(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeDecodeReferenceOrigins.String(),
DependsOn: job.IDs{metaId},
Type: op.OpTypeDecodeReferenceOrigins.String(),
DependsOn: job.IDs{metaId},
IgnoreState: ignoreState,
})
if err != nil {
return ids, err
Expand Down
12 changes: 7 additions & 5 deletions internal/indexer/document_open.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,17 @@ func (idx *Indexer) DocumentOpened(modHandle document.DirHandle) (job.IDs, error
parseId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.ParseModuleConfiguration(idx.fs, idx.modStore, modHandle.Path())
return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, modHandle.Path())
},
Type: op.OpTypeParseModuleConfiguration.String(),
Type: op.OpTypeParseModuleConfiguration.String(),
IgnoreState: true,
})
if err != nil {
return ids, err
}
ids = append(ids, parseId)

modIds, err := idx.decodeModule(modHandle, job.IDs{parseId})
modIds, err := idx.decodeModule(modHandle, job.IDs{parseId}, true)
if err != nil {
return ids, err
}
Expand All @@ -58,9 +59,10 @@ func (idx *Indexer) DocumentOpened(modHandle document.DirHandle) (job.IDs, error
parseVarsId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.ParseVariables(idx.fs, idx.modStore, modHandle.Path())
return module.ParseVariables(ctx, idx.fs, idx.modStore, modHandle.Path())
},
Type: op.OpTypeParseVariables.String(),
Type: op.OpTypeParseVariables.String(),
IgnoreState: true,
})
if err != nil {
return ids, err
Expand Down
36 changes: 21 additions & 15 deletions internal/indexer/module_calls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation"
)

func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (job.IDs, error) {
func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle, ignoreState bool) (job.IDs, error) {
jobIds := make(job.IDs, 0)

moduleCalls, err := idx.modStore.ModuleCalls(modHandle.Path())
Expand Down Expand Up @@ -43,9 +43,10 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo
parseId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: mcHandle,
Func: func(ctx context.Context) error {
return module.ParseModuleConfiguration(idx.fs, idx.modStore, mcPath)
return module.ParseModuleConfiguration(ctx, idx.fs, idx.modStore, mcPath)
},
Type: op.OpTypeParseModuleConfiguration.String(),
Type: op.OpTypeParseModuleConfiguration.String(),
IgnoreState: ignoreState,
})
if err != nil {
multierror.Append(errs, err)
Expand All @@ -60,9 +61,10 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo
Dir: mcHandle,
Type: op.OpTypeLoadModuleMetadata.String(),
Func: func(ctx context.Context) error {
return module.LoadModuleMetadata(idx.modStore, mcPath)
return module.LoadModuleMetadata(ctx, idx.modStore, mcPath)
},
DependsOn: job.IDs{parseId},
DependsOn: job.IDs{parseId},
IgnoreState: ignoreState,
})
if err != nil {
multierror.Append(errs, err)
Expand All @@ -73,7 +75,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo
}

if parseId != "" {
ids, err := idx.collectReferences(mcHandle, refCollectionDeps)
ids, err := idx.collectReferences(mcHandle, refCollectionDeps, ignoreState)
if err != nil {
multierror.Append(errs, err)
} else {
Expand All @@ -84,9 +86,10 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo
varsParseId, err := idx.jobStore.EnqueueJob(job.Job{
Dir: mcHandle,
Func: func(ctx context.Context) error {
return module.ParseVariables(idx.fs, idx.modStore, mcPath)
return module.ParseVariables(ctx, idx.fs, idx.modStore, mcPath)
},
Type: op.OpTypeParseVariables.String(),
Type: op.OpTypeParseVariables.String(),
IgnoreState: ignoreState,
})
if err != nil {
multierror.Append(errs, err)
Expand All @@ -100,8 +103,9 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo
Func: func(ctx context.Context) error {
return module.DecodeVarsReferences(ctx, idx.modStore, idx.schemaStore, mcPath)
},
Type: op.OpTypeDecodeVarsReferences.String(),
DependsOn: job.IDs{varsParseId},
Type: op.OpTypeDecodeVarsReferences.String(),
DependsOn: job.IDs{varsParseId},
IgnoreState: ignoreState,
})
if err != nil {
multierror.Append(errs, err)
Expand All @@ -114,7 +118,7 @@ func (idx *Indexer) decodeInstalledModuleCalls(modHandle document.DirHandle) (jo
return jobIds, errs.ErrorOrNil()
}

func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn job.IDs) (job.IDs, error) {
func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn job.IDs, ignoreState bool) (job.IDs, error) {
ids := make(job.IDs, 0)

var errs *multierror.Error
Expand All @@ -124,8 +128,9 @@ func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn jo
Func: func(ctx context.Context) error {
return module.DecodeReferenceTargets(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeDecodeReferenceTargets.String(),
DependsOn: dependsOn,
Type: op.OpTypeDecodeReferenceTargets.String(),
DependsOn: dependsOn,
IgnoreState: ignoreState,
})
if err != nil {
errs = multierror.Append(errs, err)
Expand All @@ -138,8 +143,9 @@ func (idx *Indexer) collectReferences(modHandle document.DirHandle, dependsOn jo
Func: func(ctx context.Context) error {
return module.DecodeReferenceOrigins(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeDecodeReferenceOrigins.String(),
DependsOn: dependsOn,
Type: op.OpTypeDecodeReferenceOrigins.String(),
DependsOn: dependsOn,
IgnoreState: ignoreState,
})
if err != nil {
errs = multierror.Append(errs, err)
Expand Down
Loading

0 comments on commit a12c9af

Please sign in to comment.