x/tools/go/packages: LoadSyntax performance after LoadMode option split into Need constants #31930

codeactual · 2019-05-09T01:54:48Z

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

$ go version
go version go1.12.5 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

My environment:

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/var/go-build"
GOEXE=""
GOFLAGS="-mod=vendor"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/var/dev/megaton/gocode"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="0"
GOMOD=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build130407124=/tmp/go-build -gno-record-gcc-switches"

Travis CI environment:

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/travis/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/travis/gopath"
GOPROXY=""
GORACE=""
GOROOT="/home/travis/.gimme/versions/go1.12.5.linux.amd64"
GOTMPDIR=""
GOTOOLDIR="/home/travis/.gimme/versions/go1.12.5.linux.amd64/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build294531687=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I upgraded from golang/tools@f727befe758c to golang/tools@fe54fb3 and noticed slower performance and out-of-memory (especially when running with CGO_ENABLED=1 for go test -race) errors in a non-public refactoring program that relies on packages.Load. I then bisected until finding golang/tools@dbeab5af.

To assist my testing I used a small program to repro which just calls packages.Load on some public packages.

In a later section I've included the output from both my local machine and a Travis build.

What did you expect to see?

I expected to see that commit golang/tools@dbeab5af did not significantly change the memory profile of programs that heavily use packages.Load (with Tests: true and Mode: LoadSyntax).

What did you see instead?

My results (local)

golang/tools@45dd101d8784, one commit prior to bisect search result:

: cat 45dd101d8784.top | head -20
File: repro
Type: alloc_space
Time: May 9, 2019 at 12:03am (UTC)
Showing nodes accounting for 170.73MB, 96.71% of 176.54MB total
Dropped 46 nodes (cum <= 0.88MB)
      flat  flat%   sum%        cum   cum%
         0     0%     0%   159.41MB 90.30%  golang.org/x/tools/go/packages.(*loader).loadPackage
         0     0%     0%   159.41MB 90.30%  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1
         0     0%     0%   159.41MB 90.30%  sync.(*Once).Do
         0     0%     0%   158.91MB 90.01%  golang.org/x/tools/go/packages.(*loader).loadRecursive
         0     0%     0%   153.74MB 87.09%  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1
         0     0%     0%   152.67MB 86.48%  golang.org/x/tools/go/gcexportdata.Read
         0     0%     0%   152.67MB 86.48%  golang.org/x/tools/go/packages.(*loader).loadFromExportData
         0     0%     0%   146.98MB 83.26%  bytes.(*Buffer).grow
  146.98MB 83.26% 83.26%   146.98MB 83.26%  bytes.makeSlice
         0     0% 83.26%   146.48MB 82.97%  bytes.(*Buffer).ReadFrom
         0     0% 83.26%   143.37MB 81.21%  io/ioutil.readAll
         0     0% 83.26%   142.86MB 80.92%  io/ioutil.ReadAll
    0.50MB  0.28% 83.54%    11.81MB  6.69%  golang.org/x/tools/go/internal/gcimporter.(*iimporter).doDecl
         0     0% 83.54%    11.81MB  6.69%  golang.org/x/tools/go/internal/gcimporter.(*importReader).obj

golang/tools@dbeab5af4b8d, result of bisect:

: cat dbeab5af4b8d.top | head -20
File: repro
Type: alloc_space
Time: May 9, 2019 at 12:03am (UTC)
Showing nodes accounting for 2647.39MB, 95.13% of 2782.91MB total
Dropped 158 nodes (cum <= 13.91MB)
      flat  flat%   sum%        cum   cum%
         0     0%     0%  1890.71MB 67.94%  go/types.(*Checker).checkFiles
         0     0%     0%  1863.15MB 66.95%  go/types.(*Checker).Files
         0     0%     0%  1842.55MB 66.21%  golang.org/x/tools/go/packages.(*loader).loadPackage
         0     0%     0%  1827.82MB 65.68%  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1
         0     0%     0%  1795.45MB 64.52%  sync.(*Once).Do
         0     0%     0%  1762.59MB 63.34%  golang.org/x/tools/go/packages.(*loader).loadRecursive
         0     0%     0%  1730.81MB 62.19%  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1
         0     0%     0%  1410.51MB 50.68%  go/types.(*Checker).rawExpr
   19.50MB   0.7%   0.7%  1302.49MB 46.80%  go/types.(*Checker).stmt
         0     0%   0.7%  1299.35MB 46.69%  go/types.(*Checker).multiExpr
         0     0%   0.7%  1298.87MB 46.67%  go/types.(*Checker).stmtList
    2.16MB 0.077%  0.78%  1275.78MB 45.84%  go/types.(*Checker).exprInternal
         0     0%  0.78%  1227.46MB 44.11%  go/types.(*Checker).funcBody
         0     0%  0.78%  1188.49MB 42.71%  go/types.(*Checker).funcDecl.func1

golang/tools@e31d36578abb, newest version at test time:

: head -20 e31d36578abb.top 
File: repro
Type: alloc_space
Time: May 9, 2019 at 1:22am (UTC)
Showing nodes accounting for 2610.92MB, 94.57% of 2760.70MB total
Dropped 167 nodes (cum <= 13.80MB)
      flat  flat%   sum%        cum   cum%
         0     0%     0%  1846.72MB 66.89%  go/types.(*Checker).checkFiles
         0     0%     0%  1822.69MB 66.02%  go/types.(*Checker).Files
    0.50MB 0.018% 0.018%  1790.19MB 64.85%  golang.org/x/tools/go/packages.(*loader).loadPackage
         0     0% 0.018%  1774.66MB 64.28%  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1
         0     0% 0.018%  1746.55MB 63.26%  sync.(*Once).Do
         0     0% 0.018%  1714.51MB 62.10%  golang.org/x/tools/go/packages.(*loader).loadRecursive
         0     0% 0.018%  1691.43MB 61.27%  golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1
         0     0% 0.018%  1389.83MB 50.34%  go/types.(*Checker).rawExpr
         0     0% 0.018%  1284.40MB 46.52%  go/types.(*Checker).multiExpr
      21MB  0.76%  0.78%  1269.81MB 46.00%  go/types.(*Checker).stmt
         0     0%  0.78%  1266.73MB 45.88%  go/types.(*Checker).stmtList
    2.60MB 0.094%  0.87%  1254.36MB 45.44%  go/types.(*Checker).exprInternal
         0     0%  0.87%  1195.09MB 43.29%  go/types.(*Checker).funcBody
         0     0%  0.87%  1155.35MB 41.85%  go/types.(*Checker).funcDecl.func1

My results (on Travis CI)

Log: https://travis-ci.org/codeactual/repro/builds/530067200#L355
Test source: https://github.com/codeactual/repro/tree/master/github/golang.org/go/x_tools_go_packages_dbeab5a
x/tools/go/packages.Config used:
- Tests: true
- Mode: LoadSyntax

It seems like the scope of LoadSyntax has significantly changed now that Load* constants are now based on the more granular Need*, especially with regard to use of go/types (based on the profiles above).

If there is a combination of Need* options which can restore similar performance with the same returned package data, I'm very interested to use them instead of LoadSyntax.

The text was updated successfully, but these errors were encountered:

andybons · 2019-05-13T19:16:02Z

@ianthehat

Before separating Load* into Need* we could use LoadSyntax to get types information by loading inital packages from source code and loading it's direct dependencies from export data. It was broken when separation was introduced and before this commit everything was loading from source code what resulted into slow loading times. This commit fixes it. Also, do backwards-incompatible fix of definition of deprecated LoadImports and LoadAllSyntax. Improve an internal error message "internal error: nil Pkg importing x from y": replace it with "internal error: package x without types was imported from y". Remove packages.NeedDeps request for loading in tests TestLoadTypesBits and TestContainsOverlayXTest. Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814, fixes golang/go#31699, golang/go#31930

Before separating Load* into Need* we could use LoadSyntax to get types information by loading inital packages from source code and loading it's direct dependencies from export data. It was broken when separation was introduced and before this commit everything was loading from source code what resulted into slow loading times. This commit fixes it. Also, do backwards-incompatible fix of definition of deprecated LoadImports and LoadAllSyntax. Improve an internal error message "internal error: nil Pkg importing x from y": replace it with "internal error: package x without types was imported from y". Remove packages.NeedDeps request for loading in tests TestLoadTypesBits and TestContainsOverlayXTest. Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814, fixes golang/go#31699, fixes golang/go#31930

gopherbot · 2019-09-09T10:42:46Z

Change https://golang.org/cl/186337 mentions this issue: go/packages: allow types loading without NeedDeps

andybons · 2019-09-09T15:25:31Z

@matloob

Before separating Load* into Need* we could use LoadSyntax to get types information by loading inital packages from source code and loading it's direct dependencies from export data. It was broken when separation was introduced and before this commit everything was loading from source code what resulted into slow loading times. This commit fixes it. Also, do backwards-incompatible fix of definition of deprecated LoadImports and LoadAllSyntax. Improve an internal error message "internal error: nil Pkg importing x from y": replace it with "internal error: package x without types was imported from y". Remove packages.NeedDeps request for loading in tests TestLoadTypesBits and TestContainsOverlayXTest. Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814, fixes golang/go#31699, fixes golang/go#31930

Before separating Load* into Need* we could use LoadSyntax to get types information by loading inital packages from source code and loading it's direct dependencies from export data. It was broken when separation was introduced and before this commit everything was loading from source code what resulted into slow loading times. This commit fixes it. Also, do backwards-incompatible fix of definition of deprecated LoadImports and LoadAllSyntax. Improve an internal error message "internal error: nil Pkg importing x from y": replace it with "internal error: package x without types was imported from y". Remove packages.NeedDeps request for loading in tests TestLoadTypesBits and TestContainsOverlayXTest. Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814, fixes golang/go#31699, fixes golang/go#31930 Change-Id: I416e3c1035d555d67039e45a4fdd1deb9b2184ef GitHub-Last-Rev: 2e3a46e GitHub-Pull-Request: golang#139 Reviewed-on: https://go-review.googlesource.com/c/tools/+/186337 Reviewed-by: Michael Matloob <matloob@golang.org>

gopherbot added this to the Unreleased milestone May 9, 2019

andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 13, 2019

codeactual mentioned this issue Aug 22, 2019

go/packages: allow types loading without NeedDeps golang/tools#139

Closed

gopherbot closed this as completed in golang/tools@7dd31af Sep 11, 2019

golang locked and limited conversation to collaborators Sep 10, 2020

gopherbot added the FrozenDueToAge label Sep 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/go/packages: LoadSyntax performance after LoadMode option split into Need constants #31930

x/tools/go/packages: LoadSyntax performance after LoadMode option split into Need constants #31930

codeactual commented May 9, 2019

andybons commented May 13, 2019

gopherbot commented Sep 9, 2019

andybons commented Sep 9, 2019

x/tools/go/packages: LoadSyntax performance after LoadMode option split into Need constants #31930

x/tools/go/packages: LoadSyntax performance after LoadMode option split into Need constants #31930

Comments

codeactual commented May 9, 2019

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

My results (local)

My results (on Travis CI)

andybons commented May 13, 2019

gopherbot commented Sep 9, 2019

andybons commented Sep 9, 2019

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

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