Closed jirfag closed 5 years ago
Related to #342 and pointers usage on latest commit.
We could dig more, sure, but it's a start.
Hi!
A current solution may be to configure GOGC
: it can trade memory to time and decrease memory usage in ~2 times. I've shown some results here.
mem.pdf Also, I've profiled memory of golangci-lint: PDF with results is attached. We can see that type checking consumes the most of memory. I have some ideas on how to fix it but they are all too costly to implement.
We have regular intermittent build failures due to high memory usage. It would be good to reduce usage.
Currently: we're using...
GOGC=20 golangci-lint run --fix --verbose --concurrency 1 --deadline 3m0s
See https://github.com/golangci/golangci-lint#memory-usage-of-golangci-lint
istio/istio repo uses 32gb to lint: INFO Memory: 218 samples, avg is 18237.6MB, max is 31795.7MB
👍 Looking forward to testing out master
I've merged a fix reducing memory usage of go/analysis linters (staticcheck, gosimple, stylecheck, bodyclose, govet) 5x. Please, check it in a master branch.
😍
bodyclose: 14.4 GB / 1m 20s -> 1.2 GB / 41s
govet: 15.5 GB / 1m 25s -> 1.8 GB / 30s
gosimple 5.9 GB / 49 s -> 1.5 GB / 40s
staticcheck: 7.2 GB / 1m 1s -> 1.7 GB / 1m 52s
govet: 15.5 GB / 1m 25s -> 1.8 GB / 30s
stylecheck: didn't run because statickcheck failed, but I'd expect similar numbers so I didn't test it
staticcheck seems to have slowed down, but it caught new errors, so maybe it was upgraded on master.
It's still using 29 GB of memory when running with all linters enabled rather than enabling 1 linter at a time. Is that expected? Is it just a side effect of Go's GC and it wouldn't use that much memory if it wasn't available?
@gracenoah thank you for comparison, I'am continuously reducing memory consumption: it's my top priority now. Is the repo rhere are you testing open-source?
It's not, sorry. It's an internal monorepo. If you want me to test some proposed performance changes, you can email me at gracenoahgh@gmail.com but that's all I can do.
I've investigated more high memory consumption.
unused
, interfacer
, unparam
. To do it we need to run them by our go/analysis
runner. Most of them require loader.Program
or AST
: we can mimic this interface from go/analysis
's *packages.Package
. It can take 1-3 weeks and reduce memory usage in the same way staticcheck
s memory dropped in v1.19.0
.unused
, interfacer
, unparam
. I will research it later.Hi @jirfag 👋 We continually have memory issues with https://github.com/terraform-providers/terraform-provider-aws if you're looking for an open source project to test. v1.19.1 still throws TravisCI out of memory errors for us with GOGC=30
, which was working okay in v1.17.1. Our issue seems to have began in v1.18.0, similar to what was noted here: https://github.com/golangci/golangci-lint/issues/731#issuecomment-534801956
Hi @jirfag - continuing the conversation here from #731!
A few hours ago, you wrote:
and reduce memory usage in the same way
staticcheck
's memory dropped inv1.19.0
But that is not the reality we are facing; in fact, we are experiencing the opposite (a huge increase in memory consumption that starts exactly at the commit claimed to improve things), as I demonstrated in #731.
Unfortunately, our codebase is not open-source, but I am happy to run experiments for you and report results back, as 6a979fb
and beyond block our ability to upgrade golangci-lint
.
One additional note for folks struggling with this. For us, a large portion of memory consumption was coming from concurrent use of go's linker. We were previously able to reduce this significantly by setting GOGC=50
, by limiting concurrency (to 4), and by running go vet
separately from golangci-lint
, but as of 6a979fb
, that strategy is no longer effective.
@bmhatfield I guess it was caused by large update of staticcheck
. I will compare it in the end with 1.17
I've made a prototype. It's results (time/peak memory usage):
repo | active linters | v1.17.1 | v1.18.0 | v1.19.1 | prototype |
---|---|---|---|---|---|
rclone | [deadcode errcheck goimports golint govet ineffassign structcheck unconvert varcheck] | 29s/5.3GB | 25s/5.3GB | 17s/1.9GB | 13s/0.67GB |
terraform-provider-aws | [deadcode errcheck gofmt gosimple govet ineffassign misspell structcheck unconvert unused varcheck] | - | - | 44s/5.8GB | 42s/2.8GB |
traefik | [deadcode depguard dogsled errcheck funlen goconst gocritic godox gofmt goimports golint gosimple govet ineffassign interfacer misspell nakedret staticcheck structcheck stylecheck typecheck unconvert unused varcheck whitespace] | 41s/6.6GB | 49s/6.7GB | 7m48s/18GB | 1m12s/2.1GB |
golangci-lint | [bodyclose deadcode depguard dogsled dupl errcheck funlen gochecknoinits goconst gocritic gocyclo godox gofmt goimports golint gosec gosimple govet ineffassign interfacer lll misspell nakedret scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace] | 17s/2.1GB | 14s/2.5GB | 24s/3.6GB | 12.6s/0.5GB |
@jirfag I happen to be at my computer just as you're posting this. I'll run against my repo with your branch right now!
Sounds fantastic, I'll try it against istio/istio today
[bhatfield go-services]% GOGC=50; golangci-lint --concurrency 4 -v run --disable=govet
INFO [config_reader] Config search paths: [./ /Users/bhatfield/Documents/digits/go-services /Users/bhatfield/Documents/digits /Users/bhatfield/Documents /Users/bhatfield /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 18 linters: [deadcode errcheck goconst gocritic gocyclo goimports gosimple ineffassign nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unused varcheck]
INFO [loader] Go packages loading at mode 575 (deps|exports_file|files|imports|name|compiled_files|types_sizes) took 3.741453781s
INFO [loader] Loaded 886 AST files in 187.128374ms
WARN [runner] Can't run linter goanalysis_metalinter: fact_deprecated: failed prerequisites: fact_deprecated@[REDACTED]/api [[REDACTED]/api/userapi.test]
INFO [runner] processing took 4.944µs with stages: nolint: 2.192µs, max_same_issues: 815ns, skip_dirs: 366ns, max_from_linter: 241ns, identifier_marker: 175ns, cgo: 154ns, autogenerated_exclude: 138ns, filename_unadjuster: 136ns, skip_files: 132ns, diff: 131ns, path_prettifier: 127ns, source_code: 61ns, path_shortener: 60ns, exclude: 55ns, uniq_by_line: 55ns, max_per_file_from_linter: 54ns, exclude-rules: 52ns
INFO [runner] linters took 10.144776527s with stages: goanalysis_metalinter: 5.966777269s, unused: 4.177935139s
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 111 samples, avg is 1409.0MB, max is 2758.4MB
INFO Execution took 14.09073157s
Seems as though we have a winner!
Current master by comparison:
[bhatfield go-services]% GOGC=50; golangci-lint --concurrency 4 -v run --disable=govet
INFO [config_reader] Config search paths: [./ /Users/bhatfield/Documents/digits/go-services /Users/bhatfield/Documents/digits /Users/bhatfield/Documents /Users/bhatfield /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 18 linters: [deadcode errcheck goconst gocritic gocyclo goimports gosimple ineffassign nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unused varcheck]
INFO [lintersdb] Optimized sublinters [staticcheck gosimple unused stylecheck] into metalinter megacheck
INFO [loader] Go packages loading at mode 991 (imports|types_sizes|compiled_files|files|name|syntax|types|types_info|deps) took 4.275296035s
INFO [runner] worker.3 took 2.108474523s with stages: gocritic: 1.644872104s, deadcode: 152.749573ms, varcheck: 130.423249ms, errcheck: 58.359607ms, unconvert: 49.271473ms, structcheck: 33.866387ms, goconst: 20.107534ms, nakedret: 18.733497ms, typecheck: 3.026µs
INFO [runner] worker.1 took 6.519319968s with stages: ineffassign: 6.365728743s, gocyclo: 55.793279ms, scopelint: 54.41237ms, prealloc: 43.334344ms
INFO [runner] worker.4 took 8.181789092s with stages: goimports: 8.18176335s
INFO [runner] worker.2 took 19.884139475s with stages: megacheck: 19.884118886s
INFO [runner] Workers idle times: #1: 13.364830687s, #3: 17.774407932s, #4: 11.702474352s
INFO [runner/skip dirs] Skipped 19 issues from dir services/internal-apis/accounting/models/vendor by pattern (^|/)vendor($|/)
INFO [runner/skip dirs] Skipped 1 issues from dir services/internal-apis/file-relay/models/vendor by pattern (^|/)vendor($|/)
INFO [runner] Issues before processing: 952, after processing: 0
INFO [runner] Processors filtering stat (out/in): identifier_marker: 799/799, exclude-rules: 0/77, path_prettifier: 952/952, skip_files: 952/952, exclude: 77/799, cgo: 952/952, skip_dirs: 932/952, autogenerated_exclude: 799/932, filename_unadjuster: 952/952
INFO [runner] processing took 121.644512ms with stages: autogenerated_exclude: 60.206428ms, exclude: 21.734738ms, skip_dirs: 21.180425ms, identifier_marker: 12.700814ms, path_prettifier: 3.165726ms, filename_unadjuster: 1.316085ms, cgo: 1.252663ms, exclude-rules: 72.081µs, nolint: 4.232µs, max_same_issues: 2.18µs, diff: 1.897µs, source_code: 1.595µs, max_from_linter: 1.322µs, path_shortener: 1.316µs, skip_files: 1.13µs, max_per_file_from_linter: 972ns, uniq_by_line: 908ns
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 106 samples, avg is 3354.2MB, max is 7537.1MB
INFO Execution took 24.269353126s
One concern is that the new branch has a warning that doesn't exist on master
:
WARN [runner] Can't run linter goanalysis_metalinter: fact_deprecated: failed prerequisites: fact_deprecated@[REDACTED]/api [[REDACTED]/api/userapi.test]
I don't know what fact_deprecated
means in this context, and userapi.test
is an entire packages worth of tests, so it's hard to track down what might be tripping up the linter.
Is it possible I'm seeing false memory decreases as a result of this warning?
@bmhatfield thank you! It's unstable yet and will try to debug this warning. Memory decrease is because of huge rework of analysis and shouldn't be affected by the warning.
Totally fair that it's unstable; completely acknowledged that I'm running from a PR branch. One other small note: the runner
INFO messages that are currently on master
also are not displaying here. So I am slightly uncertain if all the linters that were previously running are in fact still running.
@bmhatfield it's ok because I've changed the linting algorithm and didn't implement timings for a new one yet. In previous versions of golangci-lint, each linter analyzed all packages serially and linters were running in parallel. In the prototype, each package is processed in parallel by all linters and after processing of each package it's unloaded from the memory (which gives memory usage decrease).
repo | active linters | v1.17.1 | v1.18.0 | v1.19.1 | prototype |
---|---|---|---|---|---|
digits/go-services (closed source, 200+ package monorepo) | [deadcode errcheck goconst gocritic gocyclo goimports gosimple ineffassign misspell nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unused varcheck] | n/a | 10s/2.23GB | 25s/8GB | 14s/2.8GB |
@bmhatfield I'm sorry it didn't become better than v1.17.1. It will be difficult without the source code but I will try to find out how to decrease memory even more
@bmhatfield you can help if you will find out which linter eats a lot of memory
By any chance, do you have suggestions for establishing that beyond enabling/disabling them one at a time?
there is no debugging mechanism for it yet because of parallel running. Disable them by half using binary search :)
Config: (on the prototype I disabled dogsled godox whitespace
, since they aren't in 1.18)
PS: For us, the real boundary line is 4GB memory consumption. Below that, assuming a reasonable running time of less than 30s, we're able to run it in our CircleCI environment. Above that, and their runners OOM the job.
So at 14s, 2.8GB, we're in great shape compared with 25s/8gb, which won't complete in our environment.
@jirfag Found the linter consuming memory/time on the prototype: unused
.
Without unused
: 6s/407.4MB
deadcode errcheck goconst gocritic gocyclo goimports gosimple ineffassign misspell nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert varcheck
With unused
: 10s/2.2GB
deadcode errcheck goconst gocritic gocyclo goimports gosimple ineffassign misspell nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unused varcheck
(note: with unused
enabled there seems to be a fairly significant variance (±1.2GB) in memory consumed from run to run; I saw as low as 2.2GB and as high as 3.4GB; with unused
disabled the variance reduced to ±50MB)
Edit: For completeness, I also performed the same comparison (unused
enabled/disabled) on current master
; it had no meaningful impact on memory consumed or running time. I believe that's expected due to the change in running strategy, but wanted to double-check anyways.
Unused
is the only linter that eats a ton of memory because it can't work incrementally. You can disable it. You can make an issue in staticcheck GitHub repository about it: I think its optimization is possible.Do you plan to release another tag ?
@jirfag thanks a lot for the recent optimizations! Golangci-lint (df4f6766baff8f2ce10ae7a6a4d81fe37b729989) now only uses 68MB RAM on my machine. Before it used all my 16GB and freezed my machine. :-)
@fho I'm glad to hear that :) @pierrre yep, it will be included in v1.20.0 in 1 week I think.
❤️you so much @jirfag
https://github.com/golangci/golangci-lint/releases/tag/v1.20.0 includes all memory optimizations
@y0ssar1an thank you :)
Even with GOGC=1
, golangci-lint
still uses more than 3 GB of RAM when analyzing Dendrite :C
Users complain about too much memory usage exceeding 8gb