diff --git a/go/analysis/passes/fieldalignment/fieldalignment.go b/go/analysis/passes/fieldalignment/fieldalignment.go new file mode 100644 index 0000000000..a47db7c351 --- /dev/null +++ b/go/analysis/passes/fieldalignment/fieldalignment.go @@ -0,0 +1,342 @@ +// Copyright 2020 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 fieldalignment defines an Analyzer that detects structs that would take less +// memory if their fields were sorted. +package fieldalignment + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/token" + "go/types" + "sort" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `find structs that would take less memory if their fields were sorted + +This analyzer find structs that can be rearranged to take less memory, and provides +a suggested edit with the optimal order. +` + +var Analyzer = &analysis.Analyzer{ + Name: "fieldalignment", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.StructType)(nil), + } + inspect.Preorder(nodeFilter, func(node ast.Node) { + var s *ast.StructType + var ok bool + if s, ok = node.(*ast.StructType); !ok { + return + } + if tv, ok := pass.TypesInfo.Types[s]; ok { + fieldalignment(pass, s, tv.Type.(*types.Struct)) + } + }) + return nil, nil +} + +var unsafePointerTyp = types.Unsafe.Scope().Lookup("Pointer").(*types.TypeName).Type() + +func fieldalignment(pass *analysis.Pass, node *ast.StructType, typ *types.Struct) { + wordSize := pass.TypesSizes.Sizeof(unsafePointerTyp) + maxAlign := pass.TypesSizes.Alignof(unsafePointerTyp) + + s := gcSizes{wordSize, maxAlign} + optimal, indexes := optimalOrder(typ, &s) + optsz, optptrs := s.Sizeof(optimal), s.ptrdata(optimal) + + var message string + if sz := s.Sizeof(typ); sz != optsz { + message = fmt.Sprintf("struct of size %d could be %d", sz, optsz) + } else if ptrs := s.ptrdata(typ); ptrs != optptrs { + message = fmt.Sprintf("struct with %d pointer bytes could be %d", ptrs, optptrs) + } else { + // Already optimal order. + return + } + + // TODO: The ast node could contain multiple field names per list item while "indexes" + // based on *types.struct contain one item per field. Until this case is handled we + // do not provide any suggested fix for it. + if len(indexes) != len(node.Fields.List) { + pass.Report(analysis.Diagnostic{ + Pos: node.Pos(), + End: node.Pos() + token.Pos(len("struct")), + Message: message, + }) + return + } + + // Sort fields according to the optimal order. + var reordered []*ast.Field + for _, index := range indexes { + reordered = append(reordered, node.Fields.List[index]) + } + + newStr := &ast.StructType{ + Fields: &ast.FieldList{ + List: reordered, + }, + } + + // Write the newly aligned struct node to get the content for suggested fixes. + var buf bytes.Buffer + if err := format.Node(&buf, token.NewFileSet(), newStr); err != nil { + return + } + + pass.Report(analysis.Diagnostic{ + Pos: node.Pos(), + End: node.Pos() + token.Pos(len("struct")), + Message: message, + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "Rearrange fields", + TextEdits: []analysis.TextEdit{{ + Pos: node.Pos(), + End: node.End(), + NewText: buf.Bytes(), + }}, + }}, + }) +} + +func optimalOrder(str *types.Struct, sizes *gcSizes) (*types.Struct, []int) { + nf := str.NumFields() + + type elem struct { + index int + alignof int64 + sizeof int64 + ptrdata int64 + } + + elems := make([]elem, nf) + for i := 0; i < nf; i++ { + field := str.Field(i) + ft := field.Type() + elems[i] = elem{ + i, + sizes.Alignof(ft), + sizes.Sizeof(ft), + sizes.ptrdata(ft), + } + } + + sort.Slice(elems, func(i, j int) bool { + ei := &elems[i] + ej := &elems[j] + + // Place zero sized objects before non-zero sized objects. + zeroi := ei.sizeof == 0 + zeroj := ej.sizeof == 0 + if zeroi != zeroj { + return zeroi + } + + // Next, place more tightly aligned objects before less tightly aligned objects. + if ei.alignof != ej.alignof { + return ei.alignof > ej.alignof + } + + // Place pointerful objects before pointer-free objects. + noptrsi := ei.ptrdata == 0 + noptrsj := ej.ptrdata == 0 + if noptrsi != noptrsj { + return noptrsj + } + + if !noptrsi { + // If both have pointers... + + // ... then place objects with less trailing + // non-pointer bytes earlier. That is, place + // the field with the most trailing + // non-pointer bytes at the end of the + // pointerful section. + traili := ei.sizeof - ei.ptrdata + trailj := ej.sizeof - ej.ptrdata + if traili != trailj { + return traili < trailj + } + } + + // Lastly, order by size. + if ei.sizeof != ej.sizeof { + return ei.sizeof > ej.sizeof + } + + return false + }) + + fields := make([]*types.Var, nf) + indexes := make([]int, nf) + for i, e := range elems { + fields[i] = str.Field(e.index) + indexes[i] = e.index + } + return types.NewStruct(fields, nil), indexes +} + +// Code below based on go/types.StdSizes. + +type gcSizes struct { + WordSize int64 + MaxAlign int64 +} + +func (s *gcSizes) Alignof(T types.Type) int64 { + // For arrays and structs, alignment is defined in terms + // of alignment of the elements and fields, respectively. + switch t := T.Underlying().(type) { + case *types.Array: + // spec: "For a variable x of array type: unsafe.Alignof(x) + // is the same as unsafe.Alignof(x[0]), but at least 1." + return s.Alignof(t.Elem()) + case *types.Struct: + // spec: "For a variable x of struct type: unsafe.Alignof(x) + // is the largest of the values unsafe.Alignof(x.f) for each + // field f of x, but at least 1." + max := int64(1) + for i, nf := 0, t.NumFields(); i < nf; i++ { + if a := s.Alignof(t.Field(i).Type()); a > max { + max = a + } + } + return max + } + a := s.Sizeof(T) // may be 0 + // spec: "For a variable x of any type: unsafe.Alignof(x) is at least 1." + if a < 1 { + return 1 + } + if a > s.MaxAlign { + return s.MaxAlign + } + return a +} + +var basicSizes = [...]byte{ + types.Bool: 1, + types.Int8: 1, + types.Int16: 2, + types.Int32: 4, + types.Int64: 8, + types.Uint8: 1, + types.Uint16: 2, + types.Uint32: 4, + types.Uint64: 8, + types.Float32: 4, + types.Float64: 8, + types.Complex64: 8, + types.Complex128: 16, +} + +func (s *gcSizes) Sizeof(T types.Type) int64 { + switch t := T.Underlying().(type) { + case *types.Basic: + k := t.Kind() + if int(k) < len(basicSizes) { + if s := basicSizes[k]; s > 0 { + return int64(s) + } + } + if k == types.String { + return s.WordSize * 2 + } + case *types.Array: + return t.Len() * s.Sizeof(t.Elem()) + case *types.Slice: + return s.WordSize * 3 + case *types.Struct: + nf := t.NumFields() + if nf == 0 { + return 0 + } + + var o int64 + max := int64(1) + for i := 0; i < nf; i++ { + ft := t.Field(i).Type() + a, sz := s.Alignof(ft), s.Sizeof(ft) + if a > max { + max = a + } + if i == nf-1 && sz == 0 && o != 0 { + sz = 1 + } + o = align(o, a) + sz + } + return align(o, max) + case *types.Interface: + return s.WordSize * 2 + } + return s.WordSize // catch-all +} + +// align returns the smallest y >= x such that y % a == 0. +func align(x, a int64) int64 { + y := x + a - 1 + return y - y%a +} + +func (s *gcSizes) ptrdata(T types.Type) int64 { + switch t := T.Underlying().(type) { + case *types.Basic: + switch t.Kind() { + case types.String, types.UnsafePointer: + return s.WordSize + } + return 0 + case *types.Chan, *types.Map, *types.Pointer, *types.Signature, *types.Slice: + return s.WordSize + case *types.Interface: + return 2 * s.WordSize + case *types.Array: + n := t.Len() + if n == 0 { + return 0 + } + a := s.ptrdata(t.Elem()) + if a == 0 { + return 0 + } + z := s.Sizeof(t.Elem()) + return (n-1)*z + a + case *types.Struct: + nf := t.NumFields() + if nf == 0 { + return 0 + } + + var o, p int64 + for i := 0; i < nf; i++ { + ft := t.Field(i).Type() + a, sz := s.Alignof(ft), s.Sizeof(ft) + fp := s.ptrdata(ft) + o = align(o, a) + if fp != 0 { + p = o + fp + } + o += sz + } + return p + } + + panic("impossible") +} diff --git a/go/analysis/passes/fieldalignment/fieldalignment_test.go b/go/analysis/passes/fieldalignment/fieldalignment_test.go new file mode 100644 index 0000000000..97cbfc1047 --- /dev/null +++ b/go/analysis/passes/fieldalignment/fieldalignment_test.go @@ -0,0 +1,13 @@ +package fieldalignment_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/fieldalignment" +) + +func TestTest(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, fieldalignment.Analyzer, "a") +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/a.go b/go/analysis/passes/fieldalignment/testdata/src/a/a.go new file mode 100644 index 0000000000..f9700cd04a --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/a.go @@ -0,0 +1,13 @@ +package a + +type Good struct { + y int32 + x byte + z byte +} + +type Bad struct { // want "struct of size 12 could be 8" + x byte + y int32 + z byte +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/a.go.golden b/go/analysis/passes/fieldalignment/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..5a9458d59e --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/a.go.golden @@ -0,0 +1,13 @@ +package a + +type Good struct { + y int32 + x byte + z byte +} + +type Bad struct { + y int32 + x byte + z byte +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/ptr_386.go b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_386.go new file mode 100644 index 0000000000..b1a4b1ac0f --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_386.go @@ -0,0 +1,33 @@ +package a + +type PointerGood struct { + P *int + buf [1000]uintptr +} + +type PointerBad struct { // want "struct with 4004 pointer bytes could be 4" + buf [1000]uintptr + P *int +} + +type PointerSorta struct { + a struct { + p *int + q uintptr + } + b struct { + p *int + q [2]uintptr + } +} + +type PointerSortaBad struct { // want "struct with 16 pointer bytes could be 12" + a struct { + p *int + q [2]uintptr + } + b struct { + p *int + q uintptr + } +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/ptr_386.go.golden b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_386.go.golden new file mode 100644 index 0000000000..4552183d15 --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_386.go.golden @@ -0,0 +1,33 @@ +package a + +type PointerGood struct { + P *int + buf [1000]uintptr +} + +type PointerBad struct { + P *int + buf [1000]uintptr +} + +type PointerSorta struct { + a struct { + p *int + q uintptr + } + b struct { + p *int + q [2]uintptr + } +} + +type PointerSortaBad struct { + b struct { + p *int + q uintptr + } + a struct { + p *int + q [2]uintptr + } +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/ptr_amd64.go b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_amd64.go new file mode 100644 index 0000000000..0fc6cce282 --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_amd64.go @@ -0,0 +1,33 @@ +package a + +type PointerGood struct { + P *int + buf [1000]uintptr +} + +type PointerBad struct { // want "struct with 8008 pointer bytes could be 8" + buf [1000]uintptr + P *int +} + +type PointerSorta struct { + a struct { + p *int + q uintptr + } + b struct { + p *int + q [2]uintptr + } +} + +type PointerSortaBad struct { // want "struct with 32 pointer bytes could be 24" + a struct { + p *int + q [2]uintptr + } + b struct { + p *int + q uintptr + } +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/ptr_amd64.go.golden b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_amd64.go.golden new file mode 100644 index 0000000000..4552183d15 --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/ptr_amd64.go.golden @@ -0,0 +1,33 @@ +package a + +type PointerGood struct { + P *int + buf [1000]uintptr +} + +type PointerBad struct { + P *int + buf [1000]uintptr +} + +type PointerSorta struct { + a struct { + p *int + q uintptr + } + b struct { + p *int + q [2]uintptr + } +} + +type PointerSortaBad struct { + b struct { + p *int + q uintptr + } + a struct { + p *int + q [2]uintptr + } +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/zero.go b/go/analysis/passes/fieldalignment/testdata/src/a/zero.go new file mode 100644 index 0000000000..4258101cc1 --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/zero.go @@ -0,0 +1,11 @@ +package a + +type ZeroGood struct { + a [0]byte + b uint32 +} + +type ZeroBad struct { // want "struct of size 8 could be 4" + a uint32 + b [0]byte +} diff --git a/go/analysis/passes/fieldalignment/testdata/src/a/zero.go.golden b/go/analysis/passes/fieldalignment/testdata/src/a/zero.go.golden new file mode 100644 index 0000000000..750f83147a --- /dev/null +++ b/go/analysis/passes/fieldalignment/testdata/src/a/zero.go.golden @@ -0,0 +1,11 @@ +package a + +type ZeroGood struct { + a [0]byte + b uint32 +} + +type ZeroBad struct { + b [0]byte + a uint32 +} diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 11b26aca1f..955aa77708 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -24,6 +24,7 @@ import ( "golang.org/x/tools/go/analysis/passes/copylock" "golang.org/x/tools/go/analysis/passes/deepequalerrors" "golang.org/x/tools/go/analysis/passes/errorsas" + "golang.org/x/tools/go/analysis/passes/fieldalignment" "golang.org/x/tools/go/analysis/passes/httpresponse" "golang.org/x/tools/go/analysis/passes/ifaceassert" "golang.org/x/tools/go/analysis/passes/loopclosure" @@ -991,6 +992,7 @@ func defaultAnalyzers() map[string]Analyzer { // Non-vet analyzers: atomicalign.Analyzer.Name: {Analyzer: atomicalign.Analyzer, Enabled: true}, deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, Enabled: true}, + fieldalignment.Analyzer.Name: {Analyzer: fieldalignment.Analyzer, Enabled: false}, sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, Enabled: true}, testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true}, unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: false},