Skip to content

Commit

Permalink
Make fine-grained hashing. (#814)
Browse files Browse the repository at this point in the history
Speed up golint: don't typecheck packages twice.
Relates: #805
  • Loading branch information
jirfag committed Oct 14, 2019
1 parent f134361 commit 48599c6
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 67 deletions.
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -17,7 +17,7 @@ require (
github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee
github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a
github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc
github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89
github.com/golangci/lint-1 v0.0.0-20191013205115-297bf364a8e0
github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca
github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770
github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -94,8 +94,8 @@ github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a h1:iR3fYXUjHCR97qWS
github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a/go.mod h1:9qCChq59u/eW8im404Q2WWTrnBUQKjpNYKMbU4M7EFU=
github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc h1:gLLhTLMk2/SutryVJ6D4VZCU3CUqr8YloG7FPIBWFpI=
github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc/go.mod h1:e5tpTHCfVze+7EpLEozzMB3eafxo2KT5veNg1k6byQU=
github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89 h1:664ewjIQUXDvinFMbAsoH2V2Yvaro/X8BoYpIMTWGXI=
github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89/go.mod h1:66R6K6P6VWk9I95jvqGxkqJxVWGFy9XlDwLwVz1RCFg=
github.com/golangci/lint-1 v0.0.0-20191013205115-297bf364a8e0 h1:MfyDlzVjl1hoaPzPD4Gpb/QgoRfSBR0jdhwGyAWwMSA=
github.com/golangci/lint-1 v0.0.0-20191013205115-297bf364a8e0/go.mod h1:66R6K6P6VWk9I95jvqGxkqJxVWGFy9XlDwLwVz1RCFg=
github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca h1:kNY3/svz5T29MYHubXix4aDDuE3RWHkPvopM/EDv/MA=
github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca/go.mod h1:tvlJhZqDe4LMs4ZHD0oMUlt9G2LWuDGoisJTBzLMV9o=
github.com/golangci/misspell v0.0.0-20180809174111-950f5d19e770 h1:EL/O5HGrF7Jaq0yNhBLucz9hTuRzj2LdwGBOaENgxIk=
Expand Down
85 changes: 61 additions & 24 deletions internal/pkgcache/pkgcache.go
Expand Up @@ -17,6 +17,14 @@ import (
"github.com/golangci/golangci-lint/pkg/timeutils"
)

type HashMode int

const (
HashModeNeedOnlySelf HashMode = iota
HashModeNeedDirectDeps
HashModeNeedAllDeps
)

// Cache is a per-package data cache. A cached data is invalidated when
// package or it's dependencies change.
type Cache struct {
Expand Down Expand Up @@ -46,7 +54,7 @@ func (c *Cache) Trim() {
})
}

func (c *Cache) Put(pkg *packages.Package, key string, data interface{}) error {
func (c *Cache) Put(pkg *packages.Package, mode HashMode, key string, data interface{}) error {
var err error
buf := &bytes.Buffer{}
c.sw.TrackStage("gob", func() {
Expand All @@ -59,7 +67,7 @@ func (c *Cache) Put(pkg *packages.Package, key string, data interface{}) error {
var aID cache.ActionID

c.sw.TrackStage("key build", func() {
aID, err = c.pkgActionID(pkg)
aID, err = c.pkgActionID(pkg, mode)
if err == nil {
subkey, subkeyErr := cache.Subkey(aID, key)
if subkeyErr != nil {
Expand All @@ -85,11 +93,11 @@ func (c *Cache) Put(pkg *packages.Package, key string, data interface{}) error {

var ErrMissing = errors.New("missing data")

func (c *Cache) Get(pkg *packages.Package, key string, data interface{}) error {
func (c *Cache) Get(pkg *packages.Package, mode HashMode, key string, data interface{}) error {
var aID cache.ActionID
var err error
c.sw.TrackStage("key build", func() {
aID, err = c.pkgActionID(pkg)
aID, err = c.pkgActionID(pkg, mode)
if err == nil {
subkey, subkeyErr := cache.Subkey(aID, key)
if subkeyErr != nil {
Expand Down Expand Up @@ -125,8 +133,8 @@ func (c *Cache) Get(pkg *packages.Package, key string, data interface{}) error {
return nil
}

func (c *Cache) pkgActionID(pkg *packages.Package) (cache.ActionID, error) {
hash, err := c.packageHash(pkg)
func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionID, error) {
hash, err := c.packageHash(pkg, mode)
if err != nil {
return cache.ActionID{}, errors.Wrap(err, "failed to get package hash")
}
Expand All @@ -144,12 +152,19 @@ func (c *Cache) pkgActionID(pkg *packages.Package) (cache.ActionID, error) {
// packageHash computes a package's hash. The hash is based on all Go
// files that make up the package, as well as the hashes of imported
// packages.
func (c *Cache) packageHash(pkg *packages.Package) (string, error) {
cachedHash, ok := c.pkgHashes.Load(pkg)
func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error) {
type hashResults map[HashMode]string
hashResI, ok := c.pkgHashes.Load(pkg)
if ok {
return cachedHash.(string), nil
hashRes := hashResI.(hashResults)
if _, ok := hashRes[mode]; !ok {
return "", fmt.Errorf("no mode %d in hash result", mode)
}
return hashRes[mode], nil
}

hashRes := hashResults{}

key, err := cache.NewHash("package hash")
if err != nil {
return "", errors.Wrap(err, "failed to make a hash")
Expand All @@ -158,13 +173,15 @@ func (c *Cache) packageHash(pkg *packages.Package) (string, error) {
fmt.Fprintf(key, "pkgpath %s\n", pkg.PkgPath)
for _, f := range pkg.CompiledGoFiles {
c.ioSem <- struct{}{}
h, err := cache.FileHash(f)
h, fErr := cache.FileHash(f)
<-c.ioSem
if err != nil {
return "", errors.Wrapf(err, "failed to calculate file %s hash", f)
if fErr != nil {
return "", errors.Wrapf(fErr, "failed to calculate file %s hash", f)
}
fmt.Fprintf(key, "file %s %x\n", f, h)
}
curSum := key.Sum()
hashRes[HashModeNeedOnlySelf] = hex.EncodeToString(curSum[:])

imps := make([]*packages.Package, 0, len(pkg.Imports))
for _, imp := range pkg.Imports {
Expand All @@ -173,20 +190,40 @@ func (c *Cache) packageHash(pkg *packages.Package) (string, error) {
sort.Slice(imps, func(i, j int) bool {
return imps[i].PkgPath < imps[j].PkgPath
})
for _, dep := range imps {
if dep.PkgPath == "unsafe" {
continue
}

depHash, err := c.packageHash(dep)
if err != nil {
return "", errors.Wrapf(err, "failed to calculate hash for dependency %s", dep.Name)
calcDepsHash := func(depMode HashMode) error {
for _, dep := range imps {
if dep.PkgPath == "unsafe" {
continue
}

depHash, depErr := c.packageHash(dep, depMode)
if depErr != nil {
return errors.Wrapf(depErr, "failed to calculate hash for dependency %s with mode %d", dep.Name, depMode)
}

fmt.Fprintf(key, "import %s %s\n", dep.PkgPath, depHash)
}
return nil
}

if err := calcDepsHash(HashModeNeedOnlySelf); err != nil {
return "", err
}

fmt.Fprintf(key, "import %s %s\n", dep.PkgPath, depHash)
curSum = key.Sum()
hashRes[HashModeNeedDirectDeps] = hex.EncodeToString(curSum[:])

if err := calcDepsHash(HashModeNeedAllDeps); err != nil {
return "", err
}
curSum = key.Sum()
hashRes[HashModeNeedAllDeps] = hex.EncodeToString(curSum[:])

if _, ok := hashRes[mode]; !ok {
return "", fmt.Errorf("invalid mode %d", mode)
}
h := key.Sum()
ret := hex.EncodeToString(h[:])
c.pkgHashes.Store(pkg, ret)
return ret, nil

c.pkgHashes.Store(pkg, hashRes)
return hashRes[mode], nil
}
2 changes: 2 additions & 0 deletions pkg/exitcodes/exitcodes.go
Expand Up @@ -30,3 +30,5 @@ var (
Code: Failure,
}
)

// 1
14 changes: 10 additions & 4 deletions pkg/golinters/goanalysis/linter.go
Expand Up @@ -11,6 +11,9 @@ import (
"sync/atomic"
"time"

"github.com/golangci/golangci-lint/pkg/timeutils"

"github.com/golangci/golangci-lint/internal/pkgcache"
"github.com/golangci/golangci-lint/pkg/logutils"

"golang.org/x/tools/go/packages"
Expand Down Expand Up @@ -332,7 +335,7 @@ func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.
}

atomic.AddInt32(&savedIssuesCount, int32(len(encodedIssues)))
if err := lintCtx.PkgCache.Put(pkg, lintResKey, encodedIssues); err != nil {
if err := lintCtx.PkgCache.Put(pkg, pkgcache.HashModeNeedAllDeps, lintResKey, encodedIssues); err != nil {
lintCtx.Log.Infof("Failed to save package %s issues (%d) to cache: %s", pkg, len(pkgIssues), err)
} else {
issuesCacheDebugf("Saved package %s issues (%d) to cache", pkg, len(pkgIssues))
Expand Down Expand Up @@ -379,7 +382,7 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
defer wg.Done()
for pkg := range pkgCh {
var pkgIssues []EncodingIssue
err := lintCtx.PkgCache.Get(pkg, lintResKey, &pkgIssues)
err := lintCtx.PkgCache.Get(pkg, pkgcache.HashModeNeedAllDeps, lintResKey, &pkgIssues)
cacheRes := pkgToCacheRes[pkg]
cacheRes.loadErr = err
if err != nil {
Expand Down Expand Up @@ -430,8 +433,11 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
}

func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Issue, error) {
runner := newRunner(cfg.getName(), lintCtx.Log.Child("goanalysis"),
lintCtx.PkgCache, lintCtx.LoadGuard, cfg.getLoadMode())
log := lintCtx.Log.Child("goanalysis")
sw := timeutils.NewStopwatch("analyzers", log)
defer sw.PrintTopStages(10)

runner := newRunner(cfg.getName(), log, lintCtx.PkgCache, lintCtx.LoadGuard, cfg.getLoadMode(), sw)

pkgs := lintCtx.Packages
if cfg.useOriginalPackages() {
Expand Down
15 changes: 11 additions & 4 deletions pkg/golinters/goanalysis/runner.go
Expand Up @@ -28,6 +28,8 @@ import (
"sync/atomic"
"time"

"github.com/golangci/golangci-lint/pkg/timeutils"

"github.com/pkg/errors"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/gcexportdata"
Expand Down Expand Up @@ -80,16 +82,19 @@ type runner struct {
loadMode LoadMode
passToPkg map[*analysis.Pass]*packages.Package
passToPkgGuard sync.Mutex
sw *timeutils.Stopwatch
}

func newRunner(prefix string, logger logutils.Log, pkgCache *pkgcache.Cache, loadGuard *load.Guard, loadMode LoadMode) *runner {
func newRunner(prefix string, logger logutils.Log, pkgCache *pkgcache.Cache, loadGuard *load.Guard,
loadMode LoadMode, sw *timeutils.Stopwatch) *runner {
return &runner{
prefix: prefix,
log: logger,
pkgCache: pkgCache,
loadGuard: loadGuard,
loadMode: loadMode,
passToPkg: map[*analysis.Pass]*packages.Package{},
sw: sw,
}
}

Expand Down Expand Up @@ -481,7 +486,9 @@ func (act *action) analyzeSafe() {
act.a.Name, act.pkg.Name, act.isInitialPkg, act.needAnalyzeSource, p), debug.Stack())
}
}()
act.analyze()
act.r.sw.TrackStage(act.a.Name, func() {
act.analyze()
})
}

func (act *action) analyze() {
Expand Down Expand Up @@ -803,13 +810,13 @@ func (act *action) persistFactsToCache() error {
factsCacheDebugf("Caching %d facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name)

key := fmt.Sprintf("%s/facts", analyzer.Name)
return act.r.pkgCache.Put(act.pkg, key, facts)
return act.r.pkgCache.Put(act.pkg, pkgcache.HashModeNeedAllDeps, key, facts)
}

func (act *action) loadPersistedFacts() bool {
var facts []Fact
key := fmt.Sprintf("%s/facts", act.a.Name)
if err := act.r.pkgCache.Get(act.pkg, key, &facts); err != nil {
if err := act.r.pkgCache.Get(act.pkg, pkgcache.HashModeNeedAllDeps, key, &facts); err != nil {
if err != pkgcache.ErrMissing {
act.r.log.Warnf("Failed to get persisted facts: %s", err)
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/golinters/golint.go
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"go/ast"
"go/token"
"go/types"
"sync"

lintAPI "github.com/golangci/lint-1"
Expand All @@ -14,9 +15,10 @@ import (
"github.com/golangci/golangci-lint/pkg/result"
)

func golintProcessPkg(minConfidence float64, files []*ast.File, fset *token.FileSet) ([]result.Issue, error) {
func golintProcessPkg(minConfidence float64, files []*ast.File, fset *token.FileSet,
typesPkg *types.Package, typesInfo *types.Info) ([]result.Issue, error) {
l := new(lintAPI.Linter)
ps, err := l.LintASTFiles(files, fset)
ps, err := l.LintPkg(files, fset, typesPkg, typesInfo)
if err != nil {
return nil, fmt.Errorf("can't lint %d files: %s", len(files), err)
}
Expand Down Expand Up @@ -57,7 +59,7 @@ func NewGolint() *goanalysis.Linter {
nil,
).WithContextSetter(func(lintCtx *linter.Context) {
analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
res, err := golintProcessPkg(lintCtx.Settings().Golint.MinConfidence, pass.Files, pass.Fset)
res, err := golintProcessPkg(lintCtx.Settings().Golint.MinConfidence, pass.Files, pass.Fset, pass.Pkg, pass.TypesInfo)
if err != nil || len(res) == 0 {
return nil, err
}
Expand All @@ -72,5 +74,5 @@ func NewGolint() *goanalysis.Linter {
}
}).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
return resIssues
}).WithLoadMode(goanalysis.LoadModeSyntax)
}).WithLoadMode(goanalysis.LoadModeTypesInfo)
}
1 change: 1 addition & 0 deletions pkg/lint/lintersdb/manager.go
Expand Up @@ -76,6 +76,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
WithPresets(linter.PresetBugs).
WithURL("https://github.com/kisielk/errcheck"),
linter.NewConfig(golinters.NewGolint()).
WithLoadForGoAnalysis().
WithPresets(linter.PresetStyle).
WithURL("https://github.com/golang/lint"),

Expand Down
42 changes: 37 additions & 5 deletions pkg/timeutils/stopwatch.go
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/golangci/golangci-lint/pkg/logutils"
)

const noStagesText = "no stages"

type Stopwatch struct {
name string
startedAt time.Time
Expand All @@ -33,11 +35,7 @@ type stageDuration struct {
d time.Duration
}

func (s *Stopwatch) sprintStages() string {
if len(s.stages) == 0 {
return "no stages"
}

func (s *Stopwatch) stageDurationsSorted() []stageDuration {
stageDurations := []stageDuration{}
for n, d := range s.stages {
stageDurations = append(stageDurations, stageDuration{
Expand All @@ -48,6 +46,16 @@ func (s *Stopwatch) sprintStages() string {
sort.Slice(stageDurations, func(i, j int) bool {
return stageDurations[i].d > stageDurations[j].d
})
return stageDurations
}

func (s *Stopwatch) sprintStages() string {
if len(s.stages) == 0 {
return noStagesText
}

stageDurations := s.stageDurationsSorted()

stagesStrings := []string{}
for _, s := range stageDurations {
stagesStrings = append(stagesStrings, fmt.Sprintf("%s: %s", s.name, s.d))
Expand All @@ -56,6 +64,22 @@ func (s *Stopwatch) sprintStages() string {
return fmt.Sprintf("stages: %s", strings.Join(stagesStrings, ", "))
}

func (s *Stopwatch) sprintTopStages(n int) string {
if len(s.stages) == 0 {
return noStagesText
}

stageDurations := s.stageDurationsSorted()

stagesStrings := []string{}
for i := 0; i < len(stageDurations) && i < n; i++ {
s := stageDurations[i]
stagesStrings = append(stagesStrings, fmt.Sprintf("%s: %s", s.name, s.d))
}

return fmt.Sprintf("top %d stages: %s", n, strings.Join(stagesStrings, ", "))
}

func (s *Stopwatch) Print() {
p := fmt.Sprintf("%s took %s", s.name, time.Since(s.startedAt))
if len(s.stages) == 0 {
Expand All @@ -74,6 +98,14 @@ func (s *Stopwatch) PrintStages() {
s.log.Infof("%s took %s with %s", s.name, stagesDuration, s.sprintStages())
}

func (s *Stopwatch) PrintTopStages(n int) {
var stagesDuration time.Duration
for _, s := range s.stages {
stagesDuration += s
}
s.log.Infof("%s took %s with %s", s.name, stagesDuration, s.sprintTopStages(n))
}

func (s *Stopwatch) TrackStage(name string, f func()) {
startedAt := time.Now()
f()
Expand Down

0 comments on commit 48599c6

Please sign in to comment.