Merge pull request #7 from golangci/feature/benchmarks-and-readme

Fill README section about performance

Author: golangci

README.md

@@ -7,7 +7,29 @@ Sponsored by [GolangCI.com](https://golangci.com): SaaS service for running lint
 
 <a href="https://golangci.com/"><img src="docs/go.png" width="250px"></a>
 
-// TOC Here at the end
+* [GolangCI-Lint](#golangci-lint)
+* [Install](#install)
+* [Quick Start](#quick-start)
+* [Comparison](#comparison)
+	 * [gometalinter](#gometalinter)
+	 * [Run Needed Linters Manually](#run-needed-linters-manually)
+* [Performance](#performance)
+	 * [Default Mode](#default-mode)
+	 * [Fast Mode](#fast-mode)
+* [Supported Linters](#supported-linters)
+	 * [Enabled By Default Linters](#enabled-by-default-linters)
+	 * [Disabled By Default Linters (-E/--enable)](#disabled-by-default-linters--e--enable)
+* [Configuration](#configuration)
+	 * [Command-Line Options](#command-line-options)
+			* [Run Options](#run-options)
+			* [Linters](#linters)
+			* [Linters Options](#linters-options)
+			* [Issues Options](#issues-options)
+			* [Output Options](#output-options)
+	 * [Configuration File](#configuration-file)
+* [False Positives](#false-positives)
+* [FAQ](#faq)
+* [Internals](#internals)
 
 # Install
 ```bash
@@ -73,7 +95,7 @@ $ golangci-lint run --disable-all -E errcheck
 # Comparison
 ## `gometalinter`
 GolangCI-Lint was created to fix next issues with `gometalinter`:
-1. Slow work: `gometalinter` usually works for minutes in average projects. GolangCI-Lint works [2-10x times faster](#benchmarks) by [reusing work](#internals).
+1. Slow work: `gometalinter` usually works for minutes in average projects. GolangCI-Lint works [2-6x times faster](#benchmarks) by [reusing work](#internals).
 2. Huge memory consumption: parallel linters don't share the same program representation and can eat `n` times more memory (`n` - concurrency). GolangCI-Lint fixes it by sharing representation.
 3. Can't set honest concurrency: if you set it to `n` it can take `n+x` threads because of forced threads in specific linters. `gometalinter` can't do anything about it, because it runs linters as black-boxes in forked processes. In GolangCI-Lint we run all linters in one process and fully control them. Configured concurrency will be honest.
 This issue is important because often you'd like to set concurrency to CPUs count minus one to save one CPU for example for IDE. It concurrency isn't correct you will have troubles using IDE while analyzing code.
@@ -89,14 +111,53 @@ This issue is important because often you'd like to set concurrency to CPUs coun
 3. It will take more time because of different usages and need of tracking of version of `n` linters.
 
 # Performance
-## Benchmarks
+## Default Mode
+We compare golangci-lint and gometalinter in default mode, but explicitly specify all linters to enable because of small differences in default configuration.
+```bash
+$ golangci-lint run --no-config --issues-exit-code=0 --deadline=30m \
+	--disable-all --enable=deadcode  --enable=gocyclo --enable=golint --enable=varcheck \
+	--enable=structcheck --enable=maligned --enable=errcheck --enable=dupl --enable=ineffassign \
+	--enable=interfacer --enable=unconvert --enable=goconst --enable=gas --enable=megacheck
+$ gometalinter --deadline=30m --vendor --cyclo-over=30 --dupl-threshold=150 \
+	--exclude=<defaul golangci-lint excludes> --skip=testdata --skip=builtin \
+	--disable-all --enable=deadcode  --enable=gocyclo --enable=golint --enable=varcheck \
+	--enable=structcheck --enable=maligned --enable=errcheck --enable=dupl --enable=ineffassign \
+	--enable=interfacer --enable=unconvert --enable=goconst --enable=gas --enable=megacheck
+	./...
 ```
-BenchmarkWithGometalinter/self_repo/gometalinter_--fast_(4098_lines_of_code)-4                30        1482617961 ns/op
-BenchmarkWithGometalinter/self_repo/golangci-lint_fast_(4098_lines_of_code)-4                100         414381899 ns/op
-BenchmarkWithGometalinter/self_repo/gometalinter_(4098_lines_of_code)-4                        1        39304954722 ns/op
-BenchmarkWithGometalinter/self_repo/golangci-lint_(4098_lines_of_code)-4                       5        8290405036 ns/op
+
+| Repository | GolangCI Lint Time | GolangCI Is Faster In |
+| ---------- | ------------------ | --------------------- |
+| self repo, 4.4 kLoC | 9.1s | 6.6x |
+| gometalinter repo, 3.8 kLoC | 5.1s | 4.9x |
+| hugo, 69 kLoC | 12.4s | 5.8x |
+| go source, 1300 kLoC | 3m15s | 1.8x |
+
+On average golangci-lint is 4.8 times faster than gometalinter. Maximum difference is in
+self repo: 6.6 times faster, minimum difference is in go source code repo: 1.8 faster.
+
+## Fast Mode
+We compare golangci-lint and gometalinter in fast mode (`--fast`), but don't use option `--fast` because it differs a little.
+Instead we configure common linters from this option.
+```bash
+$ golangci-lint run --no-config --issues-exit-code=0 --deadline=30m \
+	--disable-all --enable=govet --enable=dupl --enable=goconst --enable=gocyclo --enable=golint --enable=ineffassign
+$ gometalinter --deadline=30m --vendor --cyclo-over=30 --dupl-threshold=150 \
+	--exclude=<defaul golangci-lint excludes> --skip=testdata --skip=builtin \
+	--disable-all --enable=vet --enable=vetshadow -enable=dupl --enable=goconst --enable=gocyclo --enable=golint --enable=ineffassign \
+	./...
 ```
-## Internals
+
+| Repository | GolangCI Lint Time | GolangCI Is Faster In |
+| ---------- | ------------------ | --------------------- |
+| self repo, 4.4 kLoC | 0.4s | 3.1x |
+| gometalinter repo, 3.8 kLoC | 0.2s | 1.9x |
+| hugo, 69 kLoC | 1.6s | 4x |
+| go source, 1300 kLoC | 35.4s | 1.17x |
+
+On average golangci-lint is 2.5 times faster than gometalinter. Maximum difference is in
+self repo: 3.1 times faster, minimum difference is in go source code repo: 17% faster.
+
 
 # Supported Linters
 To see a list of supported linters and which linters are enabled/disabled by default execute command
@@ -261,3 +322,21 @@ A: You have 2 choices:
 1. Update it: `go get -u gopkg.in/golangci/golangci-lint.v1/cmd/golangci-lint`
 2. Run it with `-v` option and check output.
 3. If it doesn't help create [GitHub issue](https://github.com/golangci/golangci-lint/issues/new).
+
+# Internals
+Key difference with gometalinter is that golangci-lint shares work between specific linters (golint, govet, ...).
+For small and medium projects 50-80% of work between linters can be reused.
+Now we share `loader.Program` and `SSA` representation building. `SSA` representation is used from
+a [fork of go-tools](https://github.com/dominikh/go-tools), not the official one. Also we are going to
+reuse `AST` parsing and traversal.
+
+We don't fork to call specific linter but use it's API. We forked github repos of almost all linters
+to make API. It also allows us to be more performant and control actual count of used threads.
+
+All linters are vendored in `/vendor` folder: their version is fixed, they are builtin
+and you don't need to install them separately.
+
+We use chains for issues and independent processors to post-process them: exclude issues by limits,
+nolint comment, diff, regexps; prettify paths etc.
+
+We use `cobra` for command-line action.

pkg/commands/root.go

@@ -41,11 +41,23 @@ func (e *Executor) initRoot() {
 			if e.cfg.Run.CPUProfilePath != "" {
 				pprof.StopCPUProfile()
 			}
+			if e.cfg.Run.MemProfilePath != "" {
+				f, err := os.Create(e.cfg.Run.MemProfilePath)
+				if err != nil {
+					log.Fatal(err)
+				}
+				runtime.GC() // get up-to-date statistics
+				if err := pprof.WriteHeapProfile(f); err != nil {
+					log.Fatal("could not write memory profile: ", err)
+				}
+			}
+
 			os.Exit(e.exitCode)
 		},
 	}
 	rootCmd.PersistentFlags().BoolVarP(&e.cfg.Run.IsVerbose, "verbose", "v", false, "verbose output")
 	rootCmd.PersistentFlags().StringVar(&e.cfg.Run.CPUProfilePath, "cpu-profile-path", "", "Path to CPU profile output file")
+	rootCmd.PersistentFlags().StringVar(&e.cfg.Run.MemProfilePath, "mem-profile-path", "", "Path to memory profile output file")
 	rootCmd.PersistentFlags().IntVarP(&e.cfg.Run.Concurrency, "concurrency", "j", runtime.NumCPU(), "Concurrency")
 
 	e.rootCmd = rootCmd

pkg/commands/run.go

@@ -56,6 +56,7 @@ func (e *Executor) initRun() {
 	runCmd.Flags().BoolVar(&rc.AnalyzeTests, "tests", false, "Analyze tests (*_test.go)")
 	runCmd.Flags().BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, "Print avg and max memory usage of golangci-lint and total time")
 	runCmd.Flags().StringVarP(&rc.Config, "config", "c", "", "Read config from file path `PATH`")
+	runCmd.Flags().BoolVar(&rc.NoConfig, "no-config", false, "Don't read config")
 
 	// Linters settings config
 	lsc := &e.cfg.LintersSettings
@@ -315,14 +316,26 @@ func (e *Executor) parseConfig(cmd *cobra.Command) {
 	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
 	viper.AutomaticEnv()
 
-	configFile := viper.GetString("config")
+	configFile := e.cfg.Run.Config
+	if e.cfg.Run.NoConfig && configFile != "" {
+		log.Fatal("can't combine option --config and --no-config")
+	}
+
+	if e.cfg.Run.NoConfig {
+		return
+	}
+
 	if configFile == "" {
 		viper.SetConfigName(".golangci")
 		viper.AddConfigPath("./")
 	} else {
 		viper.SetConfigFile(configFile)
 	}
 
+	e.parseConfigImpl()
+}
+
+func (e *Executor) parseConfigImpl() {
 	commandLineConfig := *e.cfg // make copy
 
 	if err := viper.ReadInConfig(); err != nil {
@@ -351,6 +364,10 @@ func (e *Executor) validateConfig(commandLineConfig *config.Config) error {
 		return errors.New("option run.cpuprofilepath in config isn't allowed")
 	}
 
+	if commandLineConfig.Run.MemProfilePath == "" && c.Run.MemProfilePath != "" {
+		return errors.New("option run.memprofilepath in config isn't allowed")
+	}
+
 	if !commandLineConfig.Run.IsVerbose && c.Run.IsVerbose {
 		return errors.New("can't set run.verbose option with config: only on command-line")
 	}

pkg/config/config.go

@@ -41,10 +41,12 @@ var DefaultExcludePatterns = []string{
 type Run struct {
 	IsVerbose           bool `mapstructure:"verbose"`
 	CPUProfilePath      string
+	MemProfilePath      string
 	Concurrency         int
 	PrintResourcesUsage bool `mapstructure:"print-resources-usage"`
 
-	Config string
+	Config   string
+	NoConfig bool
 
 	Args []string
 

pkg/enabled_linters_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/shirou/gopsutil/mem"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -136,80 +137,59 @@ func getBenchFastLintersArgs() []string {
 	}
 }
 
-func runGometalinter(b *testing.B) {
-	args := []string{"--disable-all", "--deadline=30m"}
-	args = append(args, getBenchLintersArgs()...)
-	args = append(args,
-		"--enable=vet",
-		"--enable=vetshadow",
+func getGometalinterCommonArgs() []string {
+	return []string{
+		"--deadline=30m",
+		"--skip=testdata",
+		"--skip=builtin",
 		"--vendor",
 		"--cyclo-over=30",
 		"--dupl-threshold=150",
 		"--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")),
-		"./...",
-	)
+		"--disable-all",
+		"--enable=vet",
+		"--enable=vetshadow",
+	}
+}
+
+func runGometalinter(b *testing.B) {
+	args := []string{}
+	args = append(args, getGometalinterCommonArgs()...)
+	args = append(args, getBenchLintersArgs()...)
+	args = append(args, "./...")
 	_ = exec.Command("gometalinter", args...).Run()
 }
 
 func runGometalinterFast(b *testing.B) {
-	args := []string{"--disable-all", "--deadline=30m"}
+	args := []string{}
+	args = append(args, getGometalinterCommonArgs()...)
 	args = append(args, getBenchFastLintersArgs()...)
-	args = append(args,
-		"--enable=vet",
-		"--enable=vetshadow",
-		"--vendor",
-		"--cyclo-over=30",
-		"--dupl-threshold=150",
-		"--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")),
-		"./...",
-	)
+	args = append(args, "./...")
 	_ = exec.Command("gometalinter", args...).Run()
 }
 
-func runGometalinterNoMegacheck(b *testing.B) {
-	args := []string{"--disable-all", "--deadline=30m"}
-	args = append(args, getBenchLintersArgsNoMegacheck()...)
-	args = append(args,
-		"--enable=vet",
-		"--enable=vetshadow",
-		"--vendor",
-		"--cyclo-over=30",
-		"--dupl-threshold=150",
-		"--exclude", fmt.Sprintf("(%s)", strings.Join(config.DefaultExcludePatterns, "|")),
-		"./...",
-	)
-	_ = exec.Command("gometalinter", args...).Run()
+func getGolangciLintCommonArgs() []string {
+	return []string{"run", "--no-config", "--issues-exit-code=0", "--deadline=30m", "--disable-all", "--enable=govet"}
 }
 
 func runGolangciLint(b *testing.B) {
-	args := []string{"run", "--issues-exit-code=0", "--disable-all", "--deadline=30m", "--enable=govet"}
+	args := getGolangciLintCommonArgs()
 	args = append(args, getBenchLintersArgs()...)
-	b.Logf("golangci-lint %s", strings.Join(args, " "))
 	out, err := exec.Command("golangci-lint", args...).CombinedOutput()
 	if err != nil {
 		b.Fatalf("can't run golangci-lint: %s, %s", err, out)
 	}
 }
 
 func runGolangciLintFast(b *testing.B) {
-	args := []string{"run", "--issues-exit-code=0", "--disable-all", "--deadline=30m", "--enable=govet"}
+	args := getGolangciLintCommonArgs()
 	args = append(args, getBenchFastLintersArgs()...)
 	out, err := exec.Command("golangci-lint", args...).CombinedOutput()
 	if err != nil {
 		b.Fatalf("can't run golangci-lint: %s, %s", err, out)
 	}
 }
 
-func runGolangciLintNoMegacheck(b *testing.B) {
-	args := []string{"run", "--issues-exit-code=0", "--disable-all", "--deadline=30m", "--enable=govet"}
-	args = append(args, getBenchLintersArgsNoMegacheck()...)
-	b.Logf("golangci-lint %s", strings.Join(args, " "))
-	out, err := exec.Command("golangci-lint", args...).CombinedOutput()
-	if err != nil {
-		b.Fatalf("can't run golangci-lint: %s, %s", err, out)
-	}
-}
-
 func getGoLinesTotalCount(b *testing.B) int {
 	cmd := exec.Command("bash", "-c", `find . -name "*.go" | fgrep -v vendor | xargs wc -l | tail -1`)
 	out, err := cmd.CombinedOutput()
@@ -276,7 +256,7 @@ func runBench(b *testing.B, run func(*testing.B), format string, args ...interfa
 	if peakUsedMemMB > startUsedMemMB {
 		linterPeakMemUsage = peakUsedMemMB - startUsedMemMB
 	}
-	b.Logf("%s: start used mem is %dMB, peak used mem is %dMB, linter peak mem usage is %dMB",
+	logrus.Warnf("%s: start used mem is %dMB, peak used mem is %dMB, linter peak mem usage is %dMB",
 		name, startUsedMemMB, peakUsedMemMB, linterPeakMemUsage)
 }
 
@@ -314,8 +294,5 @@ func BenchmarkWithGometalinter(b *testing.B) {
 
 		runBench(b, runGometalinter, "%s/gometalinter (%d lines of code)", bc.name, lc)
 		runBench(b, runGolangciLint, "%s/golangci-lint (%d lines of code)", bc.name, lc)
-
-		runBench(b, runGometalinterNoMegacheck, "%s/gometalinter wo megacheck (%d lines of code)", bc.name, lc)
-		runBench(b, runGolangciLintNoMegacheck, "%s/golangci-lint wo megacheck (%d lines of code)", bc.name, lc)
 	}
 }

pkg/golinters/golint.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/golang/lint"
 	"github.com/golangci/golangci-lint/pkg/result"
+	"github.com/sirupsen/logrus"
 )
 
 type Golint struct{}
@@ -21,14 +22,18 @@ func (Golint) Desc() string {
 
 func (g Golint) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) {
 	var issues []result.Issue
+	var lintErr error
 	for _, pkgFiles := range lintCtx.Paths.FilesGrouppedByDirs() {
 		i, err := g.lintFiles(lintCtx.Settings().Golint.MinConfidence, pkgFiles...)
 		if err != nil {
-			// TODO: skip and warn
-			return nil, fmt.Errorf("can't lint files %s: %s", lintCtx.Paths.Files, err)
+			lintErr = err
+			continue
 		}
 		issues = append(issues, i...)
 	}
+	if lintErr != nil {
+		logrus.Warnf("golint: %s", lintErr)
+	}
 
 	return issues, nil
 }

pkg/runner.go

@@ -203,8 +203,6 @@ func setOutputToDevNull() (savedStdout, savedStderr *os.File) {
 }
 
 func (r SimpleRunner) Run(ctx context.Context, linters []Linter, lintCtx *golinters.Context) <-chan result.Issue {
-	defer timeutils.NewStopwatch("runner").Print()
-
 	lintResultsCh := r.runWorkers(ctx, lintCtx, linters)
 	processedLintResultsCh := r.processLintResults(ctx, lintResultsCh)
 	if ctx.Err() != nil {

pkg/timeutils/stopwatch.go

@@ -61,7 +61,11 @@ func (s *Stopwatch) Print() {
 }
 
 func (s *Stopwatch) PrintStages() {
-	logrus.Infof("%s %s", s.name, s.sprintStages())
+	var stagesDuration time.Duration
+	for _, s := range s.stages {
+		stagesDuration += s
+	}
+	logrus.Infof("%s took %s with %s", s.name, stagesDuration, s.sprintStages())
 }
 
 func (s *Stopwatch) TrackStage(name string, f func()) {