Skip to content

Commit

Permalink
go/types, types2: optimize instance lookup in LookupFieldOrMethod
Browse files Browse the repository at this point in the history
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 golang#53992

Change-Id: I10b6deb3819ab562dbbe1913f12b977cf956dd50
Reviewed-on: https://go-review.googlesource.com/c/go/+/423935
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
  • Loading branch information
findleyr authored and romaindoumenc committed Nov 3, 2022
1 parent 3a091df commit 9d80f7b
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 2 deletions.
16 changes: 15 additions & 1 deletion src/cmd/compile/internal/types2/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,18 @@ func lookupType(m map[Type]int, typ Type) (int, bool) {
}

type instanceLookup struct {
m map[*Named][]*Named
// buf is used to avoid allocating the map m in the common case of a small
// number of instances.
buf [3]*Named
m map[*Named][]*Named
}

func (l *instanceLookup) lookup(inst *Named) *Named {
for _, t := range l.buf {
if t != nil && Identical(inst, t) {
return t
}
}
for _, t := range l.m[inst.Origin()] {
if Identical(inst, t) {
return t
Expand All @@ -274,6 +282,12 @@ func (l *instanceLookup) lookup(inst *Named) *Named {
}

func (l *instanceLookup) add(inst *Named) {
for i, t := range l.buf {
if t == nil {
l.buf[i] = inst
return
}
}
if l.m == nil {
l.m = make(map[*Named][]*Named)
}
Expand Down
55 changes: 55 additions & 0 deletions src/cmd/compile/internal/types2/lookup_test.go
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()
}
}
16 changes: 15 additions & 1 deletion src/go/types/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,18 @@ func lookupType(m map[Type]int, typ Type) (int, bool) {
}

type instanceLookup struct {
m map[*Named][]*Named
// buf is used to avoid allocating the map m in the common case of a small
// number of instances.
buf [3]*Named
m map[*Named][]*Named
}

func (l *instanceLookup) lookup(inst *Named) *Named {
for _, t := range l.buf {
if t != nil && Identical(inst, t) {
return t
}
}
for _, t := range l.m[inst.Origin()] {
if Identical(inst, t) {
return t
Expand All @@ -274,6 +282,12 @@ func (l *instanceLookup) lookup(inst *Named) *Named {
}

func (l *instanceLookup) add(inst *Named) {
for i, t := range l.buf {
if t == nil {
l.buf[i] = inst
return
}
}
if l.m == nil {
l.m = make(map[*Named][]*Named)
}
Expand Down
58 changes: 58 additions & 0 deletions src/go/types/lookup_test.go
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()
}
}

0 comments on commit 9d80f7b

Please sign in to comment.