From f1d5252456df50ecbf20a3fc80c154067911e925 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Sun, 31 Mar 2024 12:31:22 -0400 Subject: [PATCH] gopls/internal/golang: Hover: show wasted % of struct space This change causes Hover to reveal the percentage of a struct type's size that is wasted due to suboptimal field ordering, if >=20%. + test, release note Fixes golang/go#66582 Change-Id: I618f68d8a277eb21c27a320c7a62cca09d8eef0a Reviewed-on: https://go-review.googlesource.com/c/tools/+/575375 LUCI-TryBot-Result: Go LUCI Reviewed-by: Suzy Mueller --- gopls/doc/release/v0.16.0.md | 4 +- gopls/internal/golang/hover.go | 126 ++++++++++++------ .../test/marker/testdata/hover/sizeoffset.txt | 28 +++- 3 files changed, 117 insertions(+), 41 deletions(-) diff --git a/gopls/doc/release/v0.16.0.md b/gopls/doc/release/v0.16.0.md index 04306e8c1f8..3b3712dae9d 100644 --- a/gopls/doc/release/v0.16.0.md +++ b/gopls/doc/release/v0.16.0.md @@ -78,7 +78,9 @@ func (s S) set(x int) { Hovering over the identifier that declares a type or struct field now displays the size information for the type, and the offset information -for the field. This information may be helpful when making space +for the field. In addition, it reports the percentage of wasted space +due to suboptimal ordering of struct fields, if this figure is 20% or +higher. This information may be helpful when making space optimizations to your data structures, or when reading assembly code. TODO: example hover image. diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go index 5e9ba4be42f..296434b3270 100644 --- a/gopls/internal/golang/hover.go +++ b/gopls/internal/golang/hover.go @@ -17,6 +17,7 @@ import ( "go/types" "io/fs" "path/filepath" + "sort" "strconv" "strings" "text/tabwriter" @@ -39,6 +40,7 @@ import ( "golang.org/x/tools/internal/aliases" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/tokeninternal" + "golang.org/x/tools/internal/typeparams" "golang.org/x/tools/internal/typesinternal" ) @@ -247,6 +249,9 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro // Compute size information for types, // and (size, offset) for struct fields. // + // Also, if a struct type's field ordering is significantly + // wasteful of space, report its optimal size. + // // This information is useful when debugging crashes or // optimizing layout. To reduce distraction, we show it only // when hovering over the declaring identifier, @@ -272,50 +277,24 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro return fmt.Sprintf("%[1]d (%#[1]x)", x) } - var data []string // {size, offset}, both optional - - // If the type has free type parameters, its size cannot be - // computed. For now, we capture panics from go/types.Sizes. - // TODO(adonovan): use newly factored typeparams.Free. - try := func(f func()) bool { - defer func() { recover() }() - f() - return true - } + path := pathEnclosingObjNode(pgf.File, pos) - // size (types and fields) - if v, ok := obj.(*types.Var); ok && v.IsField() || is[*types.TypeName](obj) { - var sz int64 - if try(func() { sz = pkg.TypesSizes().Sizeof(obj.Type()) }) { - data = append(data, "size="+format(sz)) + // Build string of form "size=... (X% wasted), offset=...". + size, wasted, offset := computeSizeOffsetInfo(pkg, path, obj) + var buf strings.Builder + if size >= 0 { + fmt.Fprintf(&buf, "size=%s", format(size)) + if wasted >= 20 { // >=20% wasted + fmt.Fprintf(&buf, " (%d%% wasted)", wasted) } } - - // offset (fields) - if v, ok := obj.(*types.Var); ok && v.IsField() { - for _, n := range pathEnclosingObjNode(pgf.File, pos) { - if n, ok := n.(*ast.StructType); ok { - t := pkg.TypesInfo().TypeOf(n).(*types.Struct) - var fields []*types.Var - for i := 0; i < t.NumFields(); i++ { - f := t.Field(i) - fields = append(fields, f) - if f == v { - var offsets []int64 - if try(func() { offsets = pkg.TypesSizes().Offsetsof(fields) }) { - if n := len(offsets); n > 0 { - data = append(data, "offset="+format(offsets[n-1])) - } - } - break - } - } - break - } + if offset >= 0 { + if buf.Len() > 0 { + buf.WriteString(", ") } + fmt.Fprintf(&buf, "offset=%s", format(offset)) } - - sizeOffset = strings.Join(data, ", ") + sizeOffset = buf.String() } var typeDecl, methods, fields string @@ -1361,3 +1340,72 @@ func promotedFields(t types.Type, from *types.Package) []promotedField { func accessibleTo(obj types.Object, pkg *types.Package) bool { return obj.Exported() || obj.Pkg() == pkg } + +// computeSizeOffsetInfo reports the size of obj (if a type or struct +// field), its wasted space percentage (if a struct type), and its +// offset (if a struct field). It returns -1 for undefined components. +func computeSizeOffsetInfo(pkg *cache.Package, path []ast.Node, obj types.Object) (size, wasted, offset int64) { + size, wasted, offset = -1, -1, -1 + + var free typeparams.Free + sizes := pkg.TypesSizes() + + // size (types and fields) + if v, ok := obj.(*types.Var); ok && v.IsField() || is[*types.TypeName](obj) { + // If the field's type has free type parameters, + // its size cannot be computed. + if !free.Has(obj.Type()) { + size = sizes.Sizeof(obj.Type()) + } + + // wasted space (struct types) + if tStruct, ok := obj.Type().Underlying().(*types.Struct); ok && is[*types.TypeName](obj) && size > 0 { + var fields []*types.Var + for i := 0; i < tStruct.NumFields(); i++ { + fields = append(fields, tStruct.Field(i)) + } + if len(fields) > 0 { + // Sort into descending (most compact) order + // and recompute size of entire struct. + sort.Slice(fields, func(i, j int) bool { + return sizes.Sizeof(fields[i].Type()) > + sizes.Sizeof(fields[j].Type()) + }) + offsets := sizes.Offsetsof(fields) + compactSize := offsets[len(offsets)-1] + sizes.Sizeof(fields[len(fields)-1].Type()) + wasted = 100 * (size - compactSize) / size + } + } + } + + // offset (fields) + if v, ok := obj.(*types.Var); ok && v.IsField() { + // Find enclosing struct type. + var tStruct *types.Struct + for _, n := range path { + if n, ok := n.(*ast.StructType); ok { + tStruct = pkg.TypesInfo().TypeOf(n).(*types.Struct) + break + } + } + if tStruct != nil { + var fields []*types.Var + for i := 0; i < tStruct.NumFields(); i++ { + f := tStruct.Field(i) + // If any preceding field's type has free type parameters, + // its offset cannot be computed. + if free.Has(f.Type()) { + break + } + fields = append(fields, f) + if f == v { + offsets := sizes.Offsetsof(fields) + offset = offsets[len(offsets)-1] + break + } + } + } + } + + return +} diff --git a/gopls/internal/test/marker/testdata/hover/sizeoffset.txt b/gopls/internal/test/marker/testdata/hover/sizeoffset.txt index cfe4b32ffdf..86c33baf8c9 100644 --- a/gopls/internal/test/marker/testdata/hover/sizeoffset.txt +++ b/gopls/internal/test/marker/testdata/hover/sizeoffset.txt @@ -7,9 +7,11 @@ Notes: - the offset of a field is undefined if it or any preceding field has undefined size/alignment. - the test's size expectations assumes a 64-bit machine. +- requires go1.22 because size information was inaccurate before. -- flags -- -skip_goarch=386 +-min_go=go1.22 -- go.mod -- module example.com @@ -18,7 +20,7 @@ go 1.18 -- a.go -- package a -type T struct { +type T struct { //@ hover("T", "T", T) a int //@ hover("a", "a", a) U U //@ hover("U", "U", U) y, z int //@ hover("y", "y", y), hover("z", "z", z) @@ -38,6 +40,30 @@ var _ struct { Gstring G[string] //@ hover("Gstring", "Gstring", Gstring) } +type wasteful struct { //@ hover("wasteful", "wasteful", wasteful) + a bool + b [2]string + c bool +} + +-- @T -- +```go +type T struct { // size=48 (0x30) + a int //@ hover("a", "a", a) + U U //@ hover("U", "U", U) + y, z int //@ hover("y", "y", y), hover("z", "z", z) +} +``` + +[`a.T` on pkg.go.dev](https://pkg.go.dev/example.com#T) +-- @wasteful -- +```go +type wasteful struct { // size=48 (0x30) (29% wasted) + a bool + b [2]string + c bool +} +``` -- @a -- ```go field a int // size=8, offset=0