Go: x/tools/cmd/goimports: performance issue with go1.11 in modules

Created on 27 Aug 2018  路  12Comments  路  Source: golang/go

What version of Go are you using (go version)?

go version go1.11 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="~/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="~/go"
GOPROXY=""
GORACE=""
GOROOT="~/.gimme/versions/go1.11.linux.amd64"
GOTMPDIR=""
GOTOOLDIR="~/.gimme/versions/go1.11.linux.amd64/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="~/appengine/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build507619798=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Compile goimports with go1.10 and go1.11, then run on google.golang.org/appengine outside of $GOPATH. Full setup:

~ $ git clone --depth=1 https://github.com/golang/appengine.git
Cloning into 'appengine'...
remote: Counting objects: 211, done.
remote: Compressing objects: 100% (194/194), done.
remote: Total 211 (delta 12), reused 103 (delta 9), pack-reused 0
Receiving objects: 100% (211/211), 365.12 KiB | 4.68 MiB/s, done.
Resolving deltas: 100% (12/12), done.
~ $ cd go/src/golang.org/x/tools/cmd/goimports/
~/go/src/golang.org/x/tools/cmd/goimports $ git pull
Already up to date.
~/go/src/golang.org/x/tools/cmd/goimports $ go version
go version go1.10.3 linux/amd64
~/go/src/golang.org/x/tools/cmd/goimports $ go build
~/go/src/golang.org/x/tools/cmd/goimports $ mv goimports ~/appengine/goimports1.10
~/go/src/golang.org/x/tools/cmd/goimports $ eval "$(gimme 1.11)"
go version go1.11 linux/amd64
~/go/src/golang.org/x/tools/cmd/goimports $ go build
~/go/src/golang.org/x/tools/cmd/goimports $ mv goimports ~/appengine/goimports1.11
~/go/src/golang.org/x/tools/cmd/goimports $ cd ~/appengine/

What did you expect to see?

~/appengine $ time ./goimports1.10 -w .

real    0m0.788s
user    0m0.856s
sys 0m0.029s

What did you see instead?

~/appengine $ time ./goimports1.11 -w .

real    0m9.400s
user    0m12.023s
sys 0m4.576s

Outside of $GOPATH, goimports compiled with go1.11 runs much slower. Inside of $GOPATH, it's basically the same:

~/go/src/google.golang.org/appengine $ time goimports1.10 -w . && time goimports1.11 -w .

real    0m0.482s
user    0m0.505s
sys 0m0.020s

real    0m0.439s
user    0m0.464s
sys 0m0.013s
FrozenDueToAge NeedsFix modules

Most helpful comment

until goimports moves to go/packages.

When will that be?

All 12 comments

/cc @bradfitz @josharian

/cc @ianthehat

This was caused by a fix to #26504 in f85125345cc5d3eb054c90bfca4bda3544d7fcab, with modules enabled it's exec'ing go list on each import to determine import path names. Temporary fix at http://golang.org/cl/132598 until goimports moves to go/packages.

@tzneal Until this change is merged, could you advise how to pull it, for folks unfamiliar with Gerrit / googlesource?

@Mitranim Click on the Download link in the middle of the screen on the right to get three different ways to fetch the patch.

until goimports moves to go/packages.

When will that be?

@math2001 this work is in progress. For progress updates this is the correct tracking issue (https://github.com/golang/go/issues/24661 is the umbrella tracking issue for "all" tools). You might also be interested in following the fortnightly update calls we have as part of the https://groups.google.com/forum/#!forum/golang-tools group.

Progress in https://golang.org/cl/142697
Mostly working, but not ready to submit.

Progress in https://golang.org/cl/142697
Mostly working, but not ready to submit.

I tried https://golang.org./cl/128362. It works only if a working directory or its parent contains go.mod. Look imports/dirs.go in the patchset for details.

I use the next script:

#!/bin/bash

argc=$#
argv=($@)

for (( j=0; j<argc; j++ )); do
    if [ ${argv[j]} == "-srcdir" ]; then
            cd $(dirname ${argv[j+1]})
            break
    fi
done

goimports.bin $*

Is this fix going to be included in 1.12?

Is this fix going to be included in 1.12?

goimports isn't part of a release.

goimports at tip now uses go/packages for modules. It won't be as fast as non-module mode, since it still has to shell out to go list, but it should be usable. Please file (new) bugs if you have problems.

Was this page helpful?
0 / 5 - 0 ratings