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

Lower or control memory usage #337

Closed
jirfag opened this issue Dec 26, 2018 · 40 comments
Closed

Lower or control memory usage #337

jirfag opened this issue Dec 26, 2018 · 40 comments
Labels
topic: memory Huge memory consumption
Milestone

Comments

@jirfag
Copy link
Member

jirfag commented Dec 26, 2018

Users complain about too much memory usage exceeding 8gb

@lopezator
Copy link
Member

Related to #342 and pointers usage on latest commit.

We could dig more, sure, but it's a start.

@jirfag
Copy link
Member Author

jirfag commented Mar 31, 2019

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.

@jirfag
Copy link
Member Author

jirfag commented Mar 31, 2019

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.

@alexec
Copy link
Contributor

alexec commented Jul 10, 2019

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

displague added a commit to displague/build that referenced this issue Aug 1, 2019
The crossplane jenkins worker node only has 10gb of RAM.  With 1.17.1 of
golangci-lint we regularly see 10gb consumed while linting in local
environments.

The changes suggested here appear to improve the overhead.
golangci/golangci-lint#337 (comment)

Prior to the bump to 1.17.1, golangci-lint was taking 6-8GB of RAM and
completing in 10s.

With the reduction in overhead to golangci-lint that this PR introduces,
linting consumes between 8-9GB of ram and completes in 30s using the same
environment.

The intent here is to continue using latest golangci-lint with the
current Jenkins configuration, sacrificing some time.  Additional
measure can be made, such as using `--fast` tests only or further increasing the
LINT_GOGC value.

https://github.com/golangci/golangci-lint#configuration
https://github.com/golangci/golangci-lint#memory-usage-of-golangci-lint

Signed-off-by: Marques Johansson <marques@upbound.io>
@howardjohn
Copy link

istio/istio repo uses 32gb to lint: INFO Memory: 218 samples, avg is 18237.6MB, max is 31795.7MB

jirfag added a commit that referenced this issue Sep 23, 2019
Set analysis pass results to nil early to garbage collect them
soon.
Memory can be reduced for the following linters:
  - staticcheck
  - stylecheck
  - gosimple
  - govet
  - bodyclose
  - any future go/analysis linter

Relates: #712, #634, #628, #598, #509, #483, #337
jirfag added a commit that referenced this issue Sep 23, 2019
Set analysis pass results to nil early to garbage collect them
soon.
Memory can be reduced for the following linters:
  - staticcheck
  - stylecheck
  - gosimple
  - govet
  - bodyclose
  - any future go/analysis linter

Relates: #712, #634, #628, #598, #509, #483, #337
@daxmc99
Copy link

daxmc99 commented Sep 23, 2019

👍
Looking forward to testing out master

@jirfag
Copy link
Member Author

jirfag commented Sep 23, 2019

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.

@gracenoah
Copy link

😍

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.

@gracenoah
Copy link

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?

@jirfag
Copy link
Member Author

jirfag commented Sep 24, 2019

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

@gracenoah
Copy link

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.

@jirfag
Copy link
Member Author

jirfag commented Sep 25, 2019

I've investigated more high memory consumption.

  1. We can dramatically reduce memory consumption of all linters except 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 staticchecks memory dropped in v1.19.0.
  2. The biggest question is what to do with unused, interfacer, unparam. I will research it later.

@bflad
Copy link
Contributor

bflad commented Sep 25, 2019

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: #731 (comment)

@bmhatfield
Copy link

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 in v1.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.

@bmhatfield
Copy link

bmhatfield commented Sep 25, 2019

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.

@jirfag
Copy link
Member Author

jirfag commented Sep 25, 2019

@bmhatfield I guess it was caused by large update of staticcheck. I will compare it in the end with 1.17

jirfag added a commit that referenced this issue Sep 30, 2019
Run all linters per package. It allows unloading package data when it's
processed. It dramatically reduces memory (and CPU because of GC) usage.

Relates: #337
jirfag added a commit that referenced this issue Sep 30, 2019
Get rid of AST cache: load AST when needed. Optimize memory allocations
for go/analysis actions.

Relates: #337
@jirfag
Copy link
Member Author

jirfag commented Sep 30, 2019

  1. Dramatically reduce memory usage #758 was merged.
  2. I've made a bit more optimizations in reduce 1.5x memory usage on large repos on repeated runs #764
  3. 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.
  4. I'm going to close this issue after merging reduce 1.5x memory usage on large repos on repeated runs #764 Please, write your comments if you think we need to fix something with memory before that.

jirfag added a commit that referenced this issue Oct 1, 2019
Get rid of AST cache: load AST when needed. Optimize memory allocations
for go/analysis actions.

Relates: #337
@jirfag jirfag closed this as completed Oct 1, 2019
@pierrre
Copy link
Contributor

pierrre commented Oct 1, 2019

Do you plan to release another tag ?

@fho
Copy link

fho commented Oct 1, 2019

@jirfag thanks a lot for the recent optimizations!
Golangci-lint (df4f676) now only uses 68MB RAM on my machine.
Before it used all my 16GB and freezed my machine. :-)

@jirfag
Copy link
Member Author

jirfag commented Oct 1, 2019

@fho I'm glad to hear that :)
@pierrre yep, it will be included in v1.20.0 in 1 week I think.

ekinanp pushed a commit to ekinanp/wash that referenced this issue Oct 3, 2019
The latest version of golangci-lint is running slower and runs out of
memory. See golangci/golangci-lint#337 for
ongoing work on improving it.

Triple the deadline to allow more time, and only run a single thread for
testing to avoid running out of memory. Also make the garbage collector
more aggressive to avoid holding onto excess memory.

Signed-off-by: Michael Smith <michael.smith@puppet.com>
@ryboe
Copy link
Contributor

ryboe commented Oct 7, 2019

❤️you so much @jirfag

@jirfag
Copy link
Member Author

jirfag commented Oct 8, 2019

https://github.com/golangci/golangci-lint/releases/tag/v1.20.0 includes all memory optimizations

@jirfag
Copy link
Member Author

jirfag commented Oct 8, 2019

@y0ssar1an thank you :)

bflad added a commit to hashicorp/terraform-provider-aws that referenced this issue Oct 8, 2019
…, add exclude-rules for unexpected SA1019 issues, replace deprecated deadline with timeout

Reference: #10236
Reference: golangci/golangci-lint#337 (comment)
k1eran added a commit to k1eran/onos-config that referenced this issue Jan 20, 2020
See golangci-lint memory improvement described at golangci/golangci-lint#337
which may help with problem on travis CI server when testing previous commit i.e.

golangci-lint run --timeout 30m
fatal error: runtime: out of memory
@bodqhrohro
Copy link

Even with GOGC=1, golangci-lint still uses more than 3 GB of RAM when analyzing Dendrite :C

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: memory Huge memory consumption
Projects
None yet
Development

No branches or pull requests