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

fix: speed up "fast" linters #4653

Merged
merged 3 commits into from
Apr 18, 2024
Merged

fix: speed up "fast" linters #4653

merged 3 commits into from
Apr 18, 2024

Conversation

ldez
Copy link
Member

@ldez ldez commented Apr 17, 2024

Currently, there are 2 problems:

  1. typecheck uses LoadModeTypesInfo and WithLoadForGoAnalysis.
    So the load mode of the linter config for the metalinter is always LoadModeTypesInfo and never LoadModeSyntax.
  2. the metalinter has a hardcoded call to WithLoadForGoAnalysis, then the types are always loaded.

As a consequence, the "fast" linters (like lll, gofmt, misspell, etc.) are slower than expected when either they are running alone or only "fast" linters are run.

This PR changes the config linter load mode of typecheck to LoadModeNone and removes WithLoadForGoAnalysis for this fake linter.
The metalinter goanalysis load mode is computed based on linters.

typecheck doesn't need an explicit load mode because it is never run alone, then at the end, the load mode will depend on the load mode of the real linters.
Same thing for the goanalysis load mode.

FYI, there are 2 load modes:

  • the config linter load mode (LoadModeTypesInfo, LoadModeSyntax, etc.)
  • the goanalysis load mode (WithLoadForGoAnalysis, WithLoadFiles)

They are used for different purposes.


Based on local tests (verbose mode), the performances are improved (~50%).

On golangci-lint code

Current version:

  • golangci-lint cache clean; golangci-lint run --enable-only misspell --enable-only lll -v
  • Execution took between 1.4s and 950ms

With the PR:

  • golangci-lint cache clean; ./golangci-lint run --enable-only misspell --enable-only lll -v
  • Execution took between 900ms and 800ms

On Traefik code

Current version:

  • golangci-lint cache clean; golangci-lint run --enable-only gofmt --enable-only misspell -v
  • Execution took between 1.3s and 1.2ms

Current version:

  • golangci-lint cache clean; ./golangci-lint run --enable-only gofmt --enable-only misspell -v
  • Execution took between 650ms and 500ms

Fixes #2914
Fixes #4639

@ldez ldez added enhancement New feature or improvement topic: speed linter: update Update the linter implementation inside golangci-lint labels Apr 17, 2024
@ldez ldez added this to the next milestone Apr 17, 2024
@ldez ldez added bug Something isn't working and removed enhancement New feature or improvement labels Apr 17, 2024
Copy link
Member

@bombsimon bombsimon left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/lint/lintersdb/manager.go Show resolved Hide resolved
@ldez ldez merged commit 15c57c1 into golangci:master Apr 18, 2024
13 checks passed
@ldez ldez deleted the fix/speed-up branch April 18, 2024 19:45
@ldez ldez modified the milestones: next, v1.58 May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linter: update Update the linter implementation inside golangci-lint topic: speed
Projects
None yet
4 participants