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 <[email protected]> Reviewed-by: Suzy Mueller <[email protected]>
golang · Apr 4, 2024 · f1d5252 · f1d5252
1 parent 951bb40
commit f1d5252
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 41 deletions.
diff --git 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
@@ -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
@@ -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