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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: buildssa analyzer is slow on github.com/aws/aws-sdk-go-v2/service/ec2 #61506

Open
adonovan opened this issue Jul 21, 2023 · 7 comments
Open

x/tools/gopls: buildssa analyzer is slow on github.com/aws/aws-sdk-go-v2/service/ec2 #61506

adonovan opened this issue Jul 21, 2023 · 7 comments
Assignees
@adonovan
Labels
gopls/performance Issues related to gopls performance (CPU, memory, etc). gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.17.0

Comments

@adonovan
Copy link
Member

adonovan commented Jul 21, 2023 •
edited

This session involves a gopls that has been modified to enable nilness.Analyzer, which itself has been modified to declare a dummy Fact that it doesn't import or export, so as to cause nilness (and thus buildssa) to be run on all dependencies. The gopls analysis driver has also been instrumented to display the wall time of each buildssa unit. Observe how expensive SSA construction is for the ec2 package, which has an extraordinary number of methods.

(This is a minimization of a performance problem reported by a user running staticcheck (whose buildir is almost identical to buildssa) on github.com/hashiform/terraform-provider-aws/internal/provider.) See #61178.

w$ git clone https://github.com/aws/aws-sdk-go-v2
w$ cd service/ec2
ec2$ (cd ~/w/xtools && go build -o x ./gopls ) && time ~/w/xtools/x --profile.cpu=prof check ./internal/customizations/unit_test.go
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/timeconv 383.709µs
2023/07/21 11:18:39 exec buildssa@net/http/internal/testcert 285.916µs
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/strings 291.541µs
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/sdk 1.026167ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/protocol/ec2query 527.208µs
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/ratelimit 1.357458ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/sync/singleflight 1.268208ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/rand 360.208µs
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/protocol/query 10.287959ms
2023/07/21 11:18:39 exec buildssa@net/http/httptest 14.392416ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws 16.287958ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/defaults 2.928167ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/configsources 4.141917ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 3.565167ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4 6.881791ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/middleware 7.872875ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/service/ec2/internal/endpoints 3.785583ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/retry 5.897459ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/transport/http 8.835125ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/aws/signer/v4 8.82425ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/service/ec2/types 25.772458ms
2023/07/21 11:18:39 exec buildssa@github.com/aws/aws-sdk-go-v2/service/internal/presigned-url 8.336917ms
2023/07/21 11:18:41 exec buildssa@github.com/aws/aws-sdk-go-v2/service/ec2 1.056296292s  <--- note
2023/07/21 11:18:41 exec buildssa@github.com/aws/aws-sdk-go-v2/service/ec2/internal/customizations_test [github.com/aws/aws-sdk-go-v2/service/ec2/internal/customizations.test] 25.270583ms

real	0m3.619s
user	0m10.746s
sys	0m1.532s

The task of this issue is to bring down the cost of buildssa so that it is proportional to the size of the package and the exportdata and factdata sizes of its direct imports, not to its transitive size. go/ssa eagerly constructs methods for every type reachable via reflection, which seems quite unnecessary.

@findleyr
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 21, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 21, 2023
@adonovan adonovan self-assigned this Jul 21, 2023
@adonovan adonovan mentioned this issue Jul 21, 2023
Closed
@adonovan adonovan added the gopls/performance Issues related to gopls performance (CPU, memory, etc). label Jul 21, 2023
@adonovan
Copy link
Member Author

adonovan commented Jul 21, 2023 •
edited

The cost of buildssa is primarily:

  • (a) that it calls CreatePackage for every package reachable through the types.Packages.Imports graph, which is far more than we need to construct ssa.Packages for; and
  • (b) that ssa.Package.build gathers runtime types aggressively for all exported members, and creates and builds all their methods. Such aggressive reachability makes sense for whole program analysis but shouldn't be built into "separate analysis" clients such as buildssa.

I think it should be possible to alter buildssa to compute a more precise set of necessary reachable things (even if that should cost an extra pass over the syntax) and then make go/ssa build only these things (possibly requiring a new internal API function, at least temporarily).

Longer term, we should audit go/ssa through the lens of separate analysis and identify all the places where it deals with cross-package dependencies so that we can reduce to a minimum the amount of stuff from other packages (direct and indirect imports) that needs to be created before building.

@adonovan adonovan mentioned this issue Jul 21, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/512136 mentions this issue: go/analysis/passes/buildssa: reduce package creation

@gopherbot
Copy link

Change https://go.dev/cl/511715 mentions this issue: go/ssa: construct methods lazily

@hyangah hyangah modified the milestones: Unreleased, gopls/v0.12.5 Jul 24, 2023
@findleyr
Copy link
Contributor

Since we can't fix this until the equivalent change is ported to staticcheck, we're not going to land this for the next release. Moving to subsequent milestone.

@findleyr findleyr modified the milestones: gopls/v0.12.5, gopls/v0.13.0 Jul 25, 2023
@findleyr
Copy link
Contributor

This unfortunately seems unlikely to get fixed for v0.14; bumping to v0.15.0.

@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.15.0 Sep 29, 2023
@gopherbot
Copy link

Change https://go.dev/cl/537478 mentions this issue: go/analysis/passes/buildssa: reduce package creation

@adonovan
Copy link
Member Author

I've made a number of changes to go/ssa to achieve this goal, that I forgot to link to this issue. Here they are:

@findleyr findleyr modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Dec 12, 2023
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adonovan adonovan

Labels
gopls/performance Issues related to gopls performance (CPU, memory, etc). gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.17.0
Development

No branches or pull requests
4 participants
@hyangah @adonovan @gopherbot @findleyr