Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cap root module loading via worker pool #256

Merged
merged 2 commits into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/apparentlymart/go-textseg v1.0.0
github.com/creachadair/jrpc2 v0.10.0
github.com/fsnotify/fsnotify v1.4.9
github.com/gammazero/workerpool v1.0.0
github.com/google/go-cmp v0.4.1
github.com/hashicorp/go-multierror v1.1.0
github.com/hashicorp/go-version v1.2.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys=
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
github.com/gammazero/deque v0.0.0-20200227231300-1e9af0e52b46 h1:iX4+rD9Fjdx8SkmSO/O5WAIX/j79ll3kuqv5VdYt9J8=
github.com/gammazero/deque v0.0.0-20200227231300-1e9af0e52b46/go.mod h1:D90+MBHVc9Sk1lJAbEVgws0eYEurY4mv2TDso3Nxh3w=
github.com/gammazero/workerpool v1.0.0 h1:MfkJc6KL0tAmjrRDS203AZz3F+84Uod9YbL8KjpcQ00=
github.com/gammazero/workerpool v1.0.0/go.mod h1:/XWO2YAUUpPi3smDlFBl0vpX0JHwUomDM/oRMwRmnSs=
github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68=
github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down
26 changes: 22 additions & 4 deletions internal/terraform/rootmodule/root_module_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"log"
"os"
"path/filepath"
"runtime"
"strings"
"time"

"github.com/gammazero/workerpool"
"github.com/hashicorp/terraform-config-inspect/tfconfig"
"github.com/hashicorp/terraform-ls/internal/terraform/discovery"
"github.com/hashicorp/terraform-ls/internal/terraform/exec"
Expand All @@ -22,6 +24,7 @@ type rootModuleManager struct {
filesystem tfconfig.FS

syncLoading bool
workerPool *workerpool.WorkerPool
logger *log.Logger

// terraform discovery
Expand All @@ -40,9 +43,14 @@ func NewRootModuleManager(fs tfconfig.FS) RootModuleManager {

func newRootModuleManager(fs tfconfig.FS) *rootModuleManager {
d := &discovery.Discovery{}

defaultSize := 3 * runtime.NumCPU()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 is the magic number I'm guessing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 is basically an educated guess. I did some simple testing as shown above with 1000 root modules in a hierarchy (which seemed to be the higher end of range reported by folks in #186 ). That effectively triggers terraform version 1000 times and I ran that on my i7 (Macbook) with 4 cores (= pool size 12) and it finished within a few seconds.

I don't expect this to be the best configuration nor the best way of testing this, but I do expect it to behave better than before just because we trade time+memory for less CPU in principle.

I do expect we will tweak the number in the near future based on the feedback from end users and further (more scoped) testing. This is also why I added the logging.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally the performance will largely depend on the time it takes to finish the loading, which after terraform version depends on the size of the schema and exec time of terraform providers schema -json alone may span from half a second to 10+ seconds in some worst case scenarios, so there is no easy answer to this I'm afraid.

We can however make it configurable and let users deal with edge cases (too slow CPU and/or too many modules) that way. Also it would be even better to just avoid calling terraform version where we don't need to as discussed in #186 (comment)

wp := workerpool.New(defaultSize)

rmm := &rootModuleManager{
rms: make([]*rootModule, 0),
filesystem: fs,
workerPool: wp,
logger: defaultLogger,
tfDiscoFunc: d.LookPath,
tfNewExecutor: exec.NewExecutor,
Expand All @@ -51,6 +59,14 @@ func newRootModuleManager(fs tfconfig.FS) *rootModuleManager {
return rmm
}

func (rmm *rootModuleManager) WorkerPoolSize() int {
return rmm.workerPool.Size()
}

func (rmm *rootModuleManager) WorkerQueueSize() int {
return rmm.workerPool.WaitingQueueSize()
}

func (rmm *rootModuleManager) defaultRootModuleFactory(ctx context.Context, dir string) (*rootModule, error) {
rm := newRootModule(rmm.filesystem, dir)

Expand Down Expand Up @@ -106,10 +122,11 @@ func (rmm *rootModuleManager) AddAndStartLoadingRootModule(ctx context.Context,
}

rmm.logger.Printf("asynchronously loading root module %s", dir)
err = rm.StartLoading()
if err != nil {
return rm, err
}
rmm.workerPool.Submit(func() {
rm := rm
err := rm.load(context.Background())
rm.setLoadErr(err)
})

return rm, nil
}
Expand Down Expand Up @@ -253,6 +270,7 @@ func (rmm *rootModuleManager) CancelLoading() {
rm.CancelLoading()
rmm.logger.Printf("loading cancelled for %s", rm.Path())
}
rmm.workerPool.Stop()
}

// rootModuleDirFromPath strips known lock file paths and filenames
Expand Down
2 changes: 2 additions & 0 deletions internal/terraform/rootmodule/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ type RootModuleManager interface {
SetTerraformExecTimeout(timeout time.Duration)

AddAndStartLoadingRootModule(ctx context.Context, dir string) (RootModule, error)
WorkerPoolSize() int
WorkerQueueSize() int
ListRootModules() RootModules
PathsToWatch() []string
RootModuleByPath(path string) (RootModule, error)
Expand Down
9 changes: 9 additions & 0 deletions langserver/handlers/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/ioutil"
"log"
"time"

"github.com/creachadair/jrpc2"
"github.com/creachadair/jrpc2/code"
Expand Down Expand Up @@ -73,6 +74,14 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) {
svc.modMgr = svc.newRootModuleManager(fs)
svc.modMgr.SetLogger(svc.logger)

svc.logger.Printf("Worker pool size set to %d", svc.modMgr.WorkerPoolSize())

tr := newTickReporter(5 * time.Second)
tr.AddReporter(func() {
svc.logger.Printf("Root modules waiting to be loaded: %d", svc.modMgr.WorkerQueueSize())
})
tr.StartReporting(svc.sessCtx)

svc.walker = svc.newWalker()

// The following is set via CLI flags, hence available in the server context
Expand Down
40 changes: 40 additions & 0 deletions langserver/handlers/tick_reporter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package handlers

import (
"context"
"time"
)

func newTickReporter(d time.Duration) *tickReporter {
return &tickReporter{
t: time.NewTicker(d),
rfs: make([]reportFunc, 0),
}
}

type reportFunc func()

type tickReporter struct {
t *time.Ticker
rfs []reportFunc
}

func (tr *tickReporter) AddReporter(f reportFunc) {
tr.rfs = append(tr.rfs, f)
}

func (tr *tickReporter) StartReporting(ctx context.Context) {
go func(ctx context.Context, tr *tickReporter) {
for {
select {
case <-ctx.Done():
tr.t.Stop()
return
case <-tr.t.C:
for _, rf := range tr.rfs {
rf()
}
}
}
}(ctx, tr)
}
26 changes: 26 additions & 0 deletions vendor/github.com/gammazero/deque/.gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions vendor/github.com/gammazero/deque/.travis.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions vendor/github.com/gammazero/deque/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 73 additions & 0 deletions vendor/github.com/gammazero/deque/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading