mvdan / gofumpt

A stricter gofmt
https://pkg.go.dev/mvdan.cc/gofumpt
BSD 3-Clause "New" or "Revised" License
3.15k stars 110 forks source link

gofumpt is very slow when `go` is behind an asdf shim #301

Closed SOF3 closed 3 months ago

SOF3 commented 4 months ago

When I run go run mvdan.cc/gofumpt -d ., it takes 1.4s for my project, but if I do go build mvdan.cc/gofumpt -o gofumpt instead, ./gofumpt -d . takes 24s to run, and 70% of time is system time.

After some debugging, it turns out that it is becomes I installed Go with asdf, and the go command resolves to ~/.asdf/shims/go by default, but when I do go run, the installed Go version is added to my $PATH and executing Go will call the native Go binary directly. This is proved by the test that running PATH=~/.asdf/installs/golang/1.20/go/bin:$PATH ./gofumpt -d . only takes around 1.4s (i.e. adding the direct links to go and gofmt to my PATH is the factor that causes the 20x slowdown).

I don't know how gofumpt works, but I can see evidnce of invoking Go when I run strace:

[pid 505270] execve("/home/chankyin/.asdf/shims/bash", ["bash", "/home/chankyin/.asdf/plugins/gol"...], 0x7ffded66d9e8 /* 64 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/.asdf/shims/go", ["go", "mod", "edit", "-json"], 0xc000398000 /* 58 vars */ <unfinished ...>
[pid 505311] execve("/home/chankyin/.cargo/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/.asdf/shims/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/.asdf/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/.nvm/versions/node/v16.17.0/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/.asdf/installs/golang/1.18/packages/bin/,/home/chankyin/.asdf/installs/golang/1.20/packages/bin//bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/go/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/.local/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/home/chankyin/sys/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/opt/tiger/toutiao/lib/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/usr/local/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505311] execve("/usr/bin/bash", ["bash", "/home/chankyin/.asdf/shims/go", "mod", "edit", "-json"], 0x7ffd301a96e0 /* 58 vars */) = 0
[pid 505311] execve("/home/chankyin/.asdf/shims/bash", ["bash", "/home/chankyin/.asdf/bin/asdf", "exec", "go", "mod", "edit", "-json"], 0x7ffdd2c813b0 /* 58 vars */) = -1 ENOENT (No such file or directory)
[pid 505327] execve("/usr/bin/grep", ["grep", "# asdf-plugin: ", "/home/chankyin/.asdf/shims/go"], 0x56327657d760 /* 61 vars */) = 0
[pid 505335] execve("/usr/bin/grep", ["grep", "# asdf-plugin: ", "/home/chankyin/.asdf/shims/go"], 0x56327657d760 /* 61 vars */ <unfinished ...>
[pid 505341] execve("/usr/bin/grep", ["grep", "# asdf-plugin: ", "/home/chankyin/.asdf/shims/go"], 0x563276580fb0 /* 61 vars */ <unfinished ...>

You may argue that this is an asdf issue rather than a Go issue, but it would be great if gofumpt could invoke gofmt through calling the Go package directly rather than depending on the operating system.

mvdan commented 3 months ago

We run go mod edit -json from each directory to be formatted to figure out what Go module each Go package belongs to. That is what you are seeing above; this is not related to us running gofmt in any way.

Indeed this appears to be a problem with your asdf setup; it appears to just make running go much slower. I'm not sure what kind of fix you expect gofumpt to do here; there is no Go API to figure out what module a package belongs to. That information is only available through executing cmd/go.

SOF3 commented 3 months ago

I had been suffering with this issue for almost two years (I used to think it was some weird OS/machine problem) and I only lately discovered the cause here by accident (refactoring the project setup to use go run gofumpt instead of $PATH/gofumpt) and noticed the sudden improvement in performance, in which case I started trying to figure out the difference and noticed the different $PATH.

I am reporting this issue mainly to help others discover this cause more easily. I don't really know what could possibly be done about it (I guess the only reaiistic "fix" is to make asdf shims faster or warn if it is invoked at high frequency?)

mvdan commented 3 months ago

My only advice would be to stop using Go version wrappers like asdf :) More often than not they will be rather slow, especially when running tools on top of them, as they will often need to invoke go multiple times in sequence. I'm fairly sure gopls would have a very similar issue.

Go these days has a way to manage versions itself - see https://go.dev/doc/toolchain. It does add a bit of overhead, but it's cached, so in the fast path it barely adds any overhead; I seem to recall it was under a few milliseconds.

Beacuse wrappers like asdf are meant to be transparent, there's really not much that gofumpt can do. Raising an issue there sounds like a good idea, although I'm not sure what would be the best solution there other than just making the wrapper fast enough that it isn't noticeable, like GOTOOLCHAIN.