Performance: filter impossible call shapes before S1038 pattern matching #1836

Background

I am the author of Chord, a Go project. While investigating CPU usage during gopls edit diagnostics, I found that some of the cost came from Staticcheck attempting full pattern matches on calls that cannot match.

This report focuses on a small optimization opportunity in S1038, after its existing package-level filtering. It is related to the hotspot discussed in #1652, rather than a request to reimplement the work already completed there. The pattern-driven filtering discussed in #1646 is also relevant.

I understand that the project does not accept pull requests. I am sharing observations, a reproducible benchmark, and a possible approach for your consideration, without requesting review or adoption of my local implementation.

Observation and possible approach

The three patterns in simple/s1038/s1038.go share these necessary conditions:

1. The outer call has one or two arguments.
2. Its last argument is a call expression, possibly parenthesized.

Once a package passes the existing package-level filter, calls such as these can still reach full pattern matching even though they cannot produce an S1038 diagnostic:

fmt.Print("text")
fmt.Print("a", "b", "c")

My local experiment adds the following checks at the start of the traversal callback, leaving all subsequent matching unchanged:

call := node.(*ast.CallExpr)
if len(call.Args) < 1 || len(call.Args) > 2 {
    return
}
if _, ok := ast.Unparen(call.Args[len(call.Args)-1]).(*ast.CallExpr); !ok {
    return
}
// Continue with the existing fmtPrintf, methSprintf, and pkgSprintf calls.

This builds no additional index and retains the existing type checks, format-string checks, and diagnostic reporting. ast.Unparen preserves the matcher's treatment of parentheses.

Isolated S1038 benchmark

• Baseline: Staticcheck v0.8.1, using its own go.mod dependencies.
• Environment: Go 1.27.1, macOS/arm64, Apple M1 Pro.
• The experimental version adds only the S1038 checks above, without any Symbol matcher or inspector changes.
• Each operation analyzes a function containing 100 target statements. Parsing, type checking, and prerequisite analyzer construction are excluded from timing.
• The input ensures that package-level filtering does not skip the analysis. The positive case asserts that each operation produces 100 diagnostics.
• Six samples per variant, alternating execution order, with -test.benchtime=100ms, compared using benchstat.

All time differences above have p=.002, n=6. These numbers describe S1038 analyzer cost on constructed inputs, not equivalent speedups for Staticcheck or gopls as a whole. The positive case also benefits because traversal encounters the inner calls, for which unnecessary matching can be skipped.

package s1038

import (
	"go/ast"
	"go/importer"
	"go/parser"
	"go/token"
	"go/types"
	"strings"
	"testing"

	"golang.org/x/tools/go/analysis"
)

func BenchmarkCallShapes(b *testing.B) {
	for _, tc := range []struct {
		name, stmt string
		want       int
	}{
		{"noncall", `fmt.Print("x");`, 0},
		{"manyargs", `fmt.Print("x", "y", "z");`, 0},
		{"nested_miss", `fmt.Print(strings.TrimSpace("x"));`, 0},
		{"hit", `fmt.Print(fmt.Sprintf("%d", 1));`, 100},
	} {
		b.Run(tc.name, func(b *testing.B) {
			fset := token.NewFileSet()
			src := `package p
import ("fmt"; "strings")
var _ = fmt.Sprintf
var _ = strings.TrimSpace
func f(){` + strings.Repeat(tc.stmt, 100) + `}`
			f, err := parser.ParseFile(fset, "p.go", src, parser.SkipObjectResolution)
			if err != nil {
				b.Fatal(err)
			}
			info := &types.Info{
				Types:      make(map[ast.Expr]types.TypeAndValue),
				Uses:       make(map[*ast.Ident]types.Object),
				Defs:       make(map[*ast.Ident]types.Object),
				Implicits:  make(map[ast.Node]types.Object),
				Selections: make(map[*ast.SelectorExpr]*types.Selection),
			}
			pkg, err := (&types.Config{Importer: importer.Default()}).Check("example.com/p", fset, []*ast.File{f}, info)
			if err != nil {
				b.Fatal(err)
			}
			n := 0
			pass := &analysis.Pass{
				Fset: fset, Files: []*ast.File{f}, Pkg: pkg, TypesInfo: info,
				ResultOf: make(map[*analysis.Analyzer]any),
				Report:   func(analysis.Diagnostic) { n++ },
			}
			var init func(*analysis.Analyzer)
			init = func(a *analysis.Analyzer) {
				if _, ok := pass.ResultOf[a]; ok {
					return
				}
				for _, dep := range a.Requires {
					init(dep)
				}
				pass.Analyzer = a
				result, err := a.Run(pass)
				if err != nil {
					b.Fatal(err)
				}
				pass.ResultOf[a] = result
			}
			for _, a := range Analyzer.Requires {
				init(a)
			}
			pass.Analyzer = Analyzer
			b.ReportAllocs()
			b.ResetTimer()
			for b.Loop() {
				n = 0
				if _, err := run(pass); err != nil {
					b.Fatal(err)
				}
				if n != tc.want {
					b.Fatalf("got %d diagnostics, want %d", n, tc.want)
				}
			}
		})
	}
}

go test -c -o /tmp/s1038-before.test ./simple/s1038
# In the experimental worktree:
go test -c -o /tmp/s1038-after.test ./simple/s1038

for round in 1 2 3 4 5 6; do
    order="before after"
    if (( round % 2 == 0 )); then order="after before"; fi
    for variant in $order; do
        "/tmp/s1038-$variant.test" \
            -test.run='^$' -test.bench='^BenchmarkCallShapes$' \
            -test.benchtime=100ms -test.benchmem \
            > "/tmp/s1038-$round-$variant.txt"
    done
done
cat /tmp/s1038-[1-6]-before.txt > /tmp/s1038-before.txt
cat /tmp/s1038-[1-6]-after.txt > /tmp/s1038-after.txt
benchstat /tmp/s1038-before.txt /tmp/s1038-after.txt

Real editing workloads: context, not isolated attribution

The motivating workload was Chord's relatively large internal/agent package, including tests. In a two-sample experiment with unsaved edits against a fixed local source snapshot, S1038 plus a separate Symbol matcher optimization reduced CPU per sequential edit by about 9.6% and cumulative allocations per edit by about 17.0%. That snapshot included uncommitted changes, so the repository's current HEAD is not an exact reproduction of this workload.

On the public Starlark repository at 3f75dec8e4039385901a30981e3703470d77e027, six samples of the same two-change combination showed a 7.35% reduction in CPU per sequential edit and a 10.24% reduction in cumulative allocations. However, median process peak RSS increased from 437.1MB to 455.9MB (p=.065), so a memory tradeoff has not been ruled out.

These results provide application context only: the combined gains cannot be attributed entirely to S1038, and reduced cumulative allocation does not imply reduced resident memory. The isolated, reproducible evidence for this issue is the S1038 benchmark above.

Validation and limitations

• Local S1038 regression cases cover parentheses, import aliases, inner variadic calls, positive fmt/log/testing cases, and non-matches with zero arguments, too many arguments, or non-call arguments. Relevant tests and race checks passed.
• In broader validation that also included the separate Symbol experiment, module-wide vet and targeted race checks passed. The full short test suite was not entirely green: the go1.0 cases for sa1019 and sa5008 failed with couldn't hash go.mod, for a file supplied only through an overlay. The unmodified baseline reproduced these failures with the same native module dependencies on both Go 1.27.1 and 1.26.3. No fixtures were changed or skips added to hide these failures.
• Full validation against the latest master and on other platforms remains outstanding. I am not claiming that every workload is regression-free.

Question

Would a local necessary-shape check be appropriate in S1038, or would it be preferable to derive such filtering from the patterns to avoid duplicating their constraints?

I hope these measurements help assess the opportunity; I do not assume that the local implementation is the preferred upstream approach.