Make fine-grained hashing. (#814)

Speed up golint: don't typecheck packages twice.
Relates: #805

go.mod

@@ -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

go.sum

@@ -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-20191013205115-297bf364a8e0 h1:MfyDlzVjl1hoaPzPD4Gpb/QgoRfSBR0jdhwGyAWwMSA=
-github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89/go.mod h1:66R6K6P6VWk9I95jvqGxkqJxVWGFy9XlDwLwVz1RCFg=
+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=

internal/pkgcache/pkgcache.go

@@ -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 {
@@ -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() {
@@ -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 {
@@ -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 {
@@ -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) {
+func (c *Cache) pkgActionID(pkg *packages.Package, mode HashMode) (cache.ActionID, error) {
-	hash, err := c.packageHash(pkg)
+	hash, err := c.packageHash(pkg, mode)
 	if err != nil {
 		return cache.ActionID{}, errors.Wrap(err, "failed to get package hash")
 	}
@@ -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) {
+func (c *Cache) packageHash(pkg *packages.Package, mode HashMode) (string, error) {
-	cachedHash, ok := c.pkgHashes.Load(pkg)
+	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")
@@ -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 {
+		if fErr != nil {
-			return "", errors.Wrapf(err, "failed to calculate file %s hash", f)
+			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 {
@@ -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)
+	calcDepsHash := func(depMode HashMode) error {
-		if err != nil {
+		for _, dep := range imps {
-			return "", errors.Wrapf(err, "failed to calculate hash for dependency %s", dep.Name)
+			if dep.PkgPath == "unsafe" {
-		}
+				continue
+			}
 
-		fmt.Fprintf(key, "import %s %s\n", dep.PkgPath, depHash)
+			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
 	}
-	h := key.Sum()
+
-	ret := hex.EncodeToString(h[:])
+	if err := calcDepsHash(HashModeNeedOnlySelf); err != nil {
-	c.pkgHashes.Store(pkg, ret)
+		return "", err
-	return ret, nil
+	}
+
+	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)
+	}
+
+	c.pkgHashes.Store(pkg, hashRes)
+	return hashRes[mode], nil
 }

pkg/exitcodes/exitcodes.go

@@ -30,3 +30,5 @@ var (
 		Code:    Failure,
 	}
 )
+
+// 1

pkg/golinters/goanalysis/linter.go

@@ -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"
@@ -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))
@@ -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 {
@@ -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"),
+	log := lintCtx.Log.Child("goanalysis")
-		lintCtx.PkgCache, lintCtx.LoadGuard, cfg.getLoadMode())
+	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() {

pkg/golinters/goanalysis/runner.go

@@ -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"
@@ -80,9 +82,11 @@ 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,
@@ -90,6 +94,7 @@ func newRunner(prefix string, logger logutils.Log, pkgCache *pkgcache.Cache, loa
 		loadGuard: loadGuard,
 		loadMode:  loadMode,
 		passToPkg: map[*analysis.Pass]*packages.Package{},
+		sw:        sw,
 	}
 }
 
@@ -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() {
@@ -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)
 		}

pkg/golinters/golint.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
+	"go/types"
 	"sync"
 
 	lintAPI "github.com/golangci/lint-1"
@@ -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)
 	}
@@ -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
 			}
@@ -72,5 +74,5 @@ func NewGolint() *goanalysis.Linter {
 		}
 	}).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
 		return resIssues
-	}).WithLoadMode(goanalysis.LoadModeSyntax)
+	}).WithLoadMode(goanalysis.LoadModeTypesInfo)
 }

pkg/lint/lintersdb/manager.go

@@ -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"),
 

pkg/timeutils/stopwatch.go

@@ -10,6 +10,8 @@ import (
 	"github.com/golangci/golangci-lint/pkg/logutils"
 )
 
+const noStagesText = "no stages"
+
 type Stopwatch struct {
 	name      string
 	startedAt time.Time
@@ -33,11 +35,7 @@ type stageDuration struct {
 	d    time.Duration
 }
 
-func (s *Stopwatch) sprintStages() string {
+func (s *Stopwatch) stageDurationsSorted() []stageDuration {
-	if len(s.stages) == 0 {
-		return "no stages"
-	}
-
 	stageDurations := []stageDuration{}
 	for n, d := range s.stages {
 		stageDurations = append(stageDurations, stageDuration{
@@ -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))
@@ -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 {
@@ -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()

vendor/github.com/golangci/lint-1/lint.go

@@ -118,10 +118,12 @@ func (l *Linter) LintFiles(files map[string][]byte) ([]Problem, error) {
 
 // LintFiles lints a set of files of a single package.
 // The argument is a map of filename to source.
-func (l *Linter) LintASTFiles(files []*ast.File, fset *token.FileSet) ([]Problem, error) {
+func (l *Linter) LintPkg(files []*ast.File, fset *token.FileSet, typesPkg *types.Package, typesInfo *types.Info) ([]Problem, error) {
 	pkg := &pkg{
-		fset:  fset,
+		fset:      fset,
-		files: make(map[string]*file),
+		files:     make(map[string]*file),
+		typesPkg:  typesPkg,
+		typesInfo: typesInfo,
 	}
 	var pkgName string
 	for _, f := range files {
@@ -193,25 +195,6 @@ type pkg struct {
 }
 
 func (p *pkg) lint() []Problem {
-	if err := p.typeCheck(); err != nil {
-		/* TODO(dsymonds): Consider reporting these errors when golint operates on entire packages.
-		if e, ok := err.(types.Error); ok {
-			pos := p.fset.Position(e.Pos)
-			conf := 1.0
-			if strings.Contains(e.Msg, "can't find import: ") {
-				// Golint is probably being run in a context that doesn't support
-				// typechecking (e.g. package files aren't found), so don't warn about it.
-				conf = 0
-			}
-			if conf > 0 {
-				p.errorfAt(pos, conf, category("typechecking"), e.Msg)
-			}
-
-			// TODO(dsymonds): Abort if !e.Soft?
-		}
-		*/
-	}
-
 	p.scanSortable()
 	p.main = p.isMain()
 

vendor/modules.txt

@@ -72,7 +72,7 @@ github.com/golangci/gofmt/gofmt
 github.com/golangci/gofmt/goimports
 # github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc
 github.com/golangci/ineffassign
-# github.com/golangci/lint-1 v0.0.0-20190930103755-fad67e08aa89
+# github.com/golangci/lint-1 v0.0.0-20191013205115-297bf364a8e0
 github.com/golangci/lint-1
 # github.com/golangci/maligned v0.0.0-20180506175553-b1d89398deca
 github.com/golangci/maligned