-
Notifications
You must be signed in to change notification settings - Fork 17.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
go/types, types2: optimize instance lookup in LookupFieldOrMethod
LookupFieldOrMethod appears as a hotspot when benchmarking gopls' auto-completion. In particular, instanceLookup.add was allocating in the common case of structs with no embedding. This is easily fixed, by using a small array in front of the map inside of instanceLookup. Do this, and additionally add a microbenchmark. The benchmark improvement is significant: name old time/op new time/op delta LookupFieldOrMethod-12 388µs ± 6% 154µs ± 3% -60.36% (p=0.000 n=10+10) name old alloc/op new alloc/op delta LookupFieldOrMethod-12 152kB ± 0% 2kB ± 0% -98.77% (p=0.000 n=9+10) name old allocs/op new allocs/op delta LookupFieldOrMethod-12 1.41k ± 0% 0.07k ± 0% -95.38% (p=0.000 n=10+10) It should also be noted that instanceLookup is used elsewhere, in particular by validType. In those contexts, the scope is not just the current type but the entire package, and so the newly added buffer is likely to simply cause extra Identical checks. Nevertheless, those checks are cheap, and on balance the improved LookupFieldOrMethod performance leads overall to improved type-checking performance. Standard library benchmark results varied by package, but type checking speed for many packages improved by ~5%, with allocations improved by ~10%. If this weren't the case we could let the caller control the buffer size, but that optimization doesn't seem necessary at this time. For example: Check/http/funcbodies/noinfo-12 71.5ms ± 4% 67.3ms ± 2% -5.90% (p=0.000 n=20+20) Check/http/funcbodies/noinfo-12 244k ± 0% 219k ± 0% -10.36% (p=0.000 n=19+19) Updates #53992 Change-Id: I10b6deb3819ab562dbbe1913f12b977cf956dd50 Reviewed-on: https://go-review.googlesource.com/c/go/+/423935 Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
- Loading branch information
Showing
4 changed files
with
143 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// Copyright 2022 The Go Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package types2_test | ||
|
||
import ( | ||
"path/filepath" | ||
"runtime" | ||
"testing" | ||
|
||
. "cmd/compile/internal/types2" | ||
) | ||
|
||
// BenchmarkLookupFieldOrMethod measures types.LookupFieldOrMethod performance. | ||
// LookupFieldOrMethod is a performance hotspot for both type-checking and | ||
// external API calls. | ||
func BenchmarkLookupFieldOrMethod(b *testing.B) { | ||
// Choose an arbitrary, large package. | ||
path := filepath.Join(runtime.GOROOT(), "src", "net", "http") | ||
|
||
files, err := pkgFiles(path) | ||
if err != nil { | ||
b.Fatal(err) | ||
} | ||
|
||
conf := Config{ | ||
Importer: defaultImporter(), | ||
} | ||
|
||
pkg, err := conf.Check("http", files, nil) | ||
if err != nil { | ||
b.Fatal(err) | ||
} | ||
|
||
scope := pkg.Scope() | ||
names := scope.Names() | ||
|
||
// Look up an arbitrary name for each type referenced in the package scope. | ||
lookup := func() { | ||
for _, name := range names { | ||
typ := scope.Lookup(name).Type() | ||
LookupFieldOrMethod(typ, true, pkg, "m") | ||
} | ||
} | ||
|
||
// Perform a lookup once, to ensure that any lazily-evaluated state is | ||
// complete. | ||
lookup() | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
lookup() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
// Copyright 2022 The Go Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package types_test | ||
|
||
import ( | ||
"go/importer" | ||
"go/token" | ||
"path/filepath" | ||
"runtime" | ||
"testing" | ||
|
||
. "go/types" | ||
) | ||
|
||
// BenchmarkLookupFieldOrMethod measures types.LookupFieldOrMethod performance. | ||
// LookupFieldOrMethod is a performance hotspot for both type-checking and | ||
// external API calls. | ||
func BenchmarkLookupFieldOrMethod(b *testing.B) { | ||
// Choose an arbitrary, large package. | ||
path := filepath.Join(runtime.GOROOT(), "src", "net", "http") | ||
|
||
fset := token.NewFileSet() | ||
files, err := pkgFiles(fset, path, 0) | ||
if err != nil { | ||
b.Fatal(err) | ||
} | ||
|
||
conf := Config{ | ||
Importer: importer.Default(), | ||
} | ||
|
||
pkg, err := conf.Check("http", fset, files, nil) | ||
if err != nil { | ||
b.Fatal(err) | ||
} | ||
|
||
scope := pkg.Scope() | ||
names := scope.Names() | ||
|
||
// Look up an arbitrary name for each type referenced in the package scope. | ||
lookup := func() { | ||
for _, name := range names { | ||
typ := scope.Lookup(name).Type() | ||
LookupFieldOrMethod(typ, true, pkg, "m") | ||
} | ||
} | ||
|
||
// Perform a lookup once, to ensure that any lazily-evaluated state is | ||
// complete. | ||
lookup() | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
lookup() | ||
} | ||
} |