|author||Daniel Martí <email@example.com>||Sat Aug 15 16:20:50 2020 +0200|
|committer||Daniel Martí <firstname.lastname@example.org>||Sat Sep 12 12:39:50 2020 +0000|
cmd/go: avoid flag.FlagSet.VisitAll at init time We want to error early if GOFLAGS contains any flag that isn't known to any cmd/go command. Thus, at init time we would recursively use VisitAll on each of the flagsets to populate a map of all registered flags. This was unfortunate, as populating said map constituted a whole 5% of the run-time of 'go env GOARCH'. This is because VisitAll is pretty expensive; it copies all the maps from the flagset's map to a slice, sorts the slice, then does one callback per flag. First, this was a bit wasteful. We only ever needed to query the knownFlag map if GOFLAGS wasn't empty. If it's empty, there's no work to do, thus we can skip the map populating work. Second and most important, we don't actually need the map at all. A flag.FlagSet already has a Lookup method, so we can simply recursively call those methods for each flag in GOFLAGS. Add a hasFlag func to make that evident. This mechanism is different; its upfront cost is none, but it will likely mean a handful of map lookups for each flag in GOFLAGS. However, that tradeoff is worth it; we don't expect GOFLAGS to contain thousands of flags. The most likely scenario is less than a dozen flags, in which case constructing a "unified" map is not at all a net win. One possible reason the previous mechanism was that way could be AddKnownFlag. Thankfully, the one and only use of that API was removed last year when Bryan cleaned up flag parsing in cmd/go. The wins for the existing benchmark with an empty GOFLAGS are significant: name old time/op new time/op delta ExecGoEnv-8 575µs ± 1% 549µs ± 2% -4.44% (p=0.000 n=7+8) name old sys-time/op new sys-time/op delta ExecGoEnv-8 1.69ms ± 1% 1.68ms ± 2% ~ (p=0.281 n=7+8) name old user-time/op new user-time/op delta ExecGoEnv-8 1.80ms ± 1% 1.66ms ± 2% -8.09% (p=0.000 n=7+8) To prove that a relatively large number of GOFLAGS isn't getting noticeably slower, we measured that as well, via benchcmd and GOFLAGS containing 50 valid flags: GOFLAGS=$(yes -- -race | sed 50q) benchcmd -n 500 GoEnvGOFLAGS go env GOARCH And the result, while noisy, shows no noticeable difference (note that it measures 3ms instead of 0.6ms since it's sequential): name old time/op new time/op delta GoEnvGOFLAGS 3.04ms ±32% 3.03ms ±35% ~ (p=0.156 n=487+481) Finally, we've improved the existing Go benchmark. Now it's parallel, and it also reports sys-time and user-time, which are useful metrics. Change-Id: I9b4551415cedf2f819eb184a02324b8bd919e2bd Reviewed-on: https://go-review.googlesource.com/c/go/+/248757 Reviewed-by: Bryan C. Mills <email@example.com> Run-TryBot: Bryan C. Mills <firstname.lastname@example.org> TryBot-Result: Gobot Gobot <email@example.com>
Go is an open source programming language that makes it easy to build simple, reliable, and efficient software.
Unless otherwise noted, the Go source files are distributed under the BSD-style license found in the LICENSE file.
Official binary distributions are available at https://golang.org/dl/.
If a binary distribution is not available for your combination of operating system and architecture, visit https://golang.org/doc/install/source or load doc/install-source.html in your web browser for source installation instructions.
Go is the work of thousands of contributors. We appreciate your help!
To contribute, please read the contribution guidelines: https://golang.org/doc/contribute.html
Note that the Go project uses the issue tracker for bug reports and proposals only. See https://golang.org/wiki/Questions for a list of places to ask questions about the Go language.