Don't shell out to go list when not needed#9776
Conversation
|
This is awesome! I can confirm the timings, but I guess this is expected given I have exact same machine 😄 Going by https://stackoverflow.com/questions/448910/what-is-the-difference-between-the-gnu-makefile-variable-assignments-a switching from But here it looks fine: there should be no difference in the results the executables return, and the single variable referenced (ADDFLAGS) is defined from the start and doesn't change value. |
| docker run \ | ||
| $(DOCKERFLAGS) -e CLANG_FORMAT=/usr/bin/clang-format-10 -t $(DEVBOX) \ | ||
| make -C /go/src/github.com/gravitational/teleport DISABLE_TEST_TARGETS=1 devbox-grpc | ||
| make -C /go/src/github.com/gravitational/teleport devbox-grpc |
There was a problem hiding this comment.
This reverts the change I made in #9432 because I thought that the performance problems were caused by some quirks in the docker vfs bindmounts, so I was only excluding the test targets for the inner part of make grpc; with the change in the makefile, this isn't necessary anymore.
I'll note this in the PR description.
|
Works fine for me and is also much faster, both natively on an Intel Mac and in a Linux VM. Nice! |
|
So awesome, thanks @espadolini!! |
6 participants
This replaces all the
:=assignments to target-local variables that are only used once with=assignments; as the=assignments are not expanded until needed, this prevents useless calls togo listorfindthat happen just as part of the makefile parsing.The time for a
make full-entwith a hot cache goes from 39.5 seconds to 3.6 seconds on my laptop; likewise, the time formake print-versiongoes from 2.8 seconds to 0.2 seconds on this machine (16" 2021 M1 Max MacBook Pro running macOS 12.1, go 1.17.6, rust 1.57.0).This reverts the change in #9432 that selectively disabled the test targets when running the makefile as part of
make grpc- the only reason that change was made was for performance, which this PR improves.