Repository navigation
analysis/callcheck: avoid formatting full function names for calls that cannot match any rule #1840
Description
Summary
While profiling gopls edit diagnostics on Chord, I found avoidable allocations in analysis/callcheck: every static callee with a *types.Func object is passed to typeutil.FuncName before looking it up in an analyzer's rules. Even when the function's unqualified name cannot match any rule, this formats its full name, including its receiver type for methods.
This is a separate hotspot from #1836, which concerns S1038 AST pattern matching. The experiment below changes only callcheck; it does not include the S1038 or Symbol matcher experiments.
I understand that this project does not accept pull requests. This report provides measurements and a reproducible benchmark for consideration, rather than requesting review or adoption of a local patch.
Observation and possible approach
For rules targeting a small set of functions/methods, reject a callee whose unqualified name is absent from that set before constructing the full name. Calls with a matching short name still go through the original full-name lookup, including receiver and package identity. IR traversal, argument extraction and diagnostic reporting remain unchanged.
The measured experiment builds the short-name set per checkCalls invocation. It adds no cross-pass cache and does not extend the lifetime of AST, type or IR objects. Rebuilding also reflects rule-map changes between invocations. The experiment assumes rules are not changed by callbacks during an invocation; it does not establish a new rule-map mutability contract.
Exact experimental production change
diff --git a/analysis/callcheck/callcheck.go b/analysis/callcheck/callcheck.go
index 3ad6888c..527ac7b2 100644
--- a/analysis/callcheck/callcheck.go
+++ b/analysis/callcheck/callcheck.go
@@ -6,6 +6,7 @@ import (
"go/ast"
"go/constant"
"go/types"
+ "strings"
"golang.org/x/tools/go/analysis"
"honnef.co/go/tools/analysis/report"
@@ -51,9 +52,15 @@ func Analyzer(rules map[string]Check) func(pass *analysis.Pass) (any, error) {
}
func checkCalls(pass *analysis.Pass, rules map[string]Check) (any, error) {
+ // FuncName may format a large receiver type. Reject names that cannot
+ // match any rule before constructing the fully qualified name.
+ names := make(map[string]bool, len(rules))
+ for name := range rules {
+ names[name[strings.LastIndexByte(name, '.')+1:]] = true
+ }
cb := func(caller *ir.Function, site ir.CallInstruction, callee *ir.Function) {
obj, ok := callee.Object().(*types.Func)
- if !ok {
+ if !ok || !names[obj.Name()] {
return
}
Isolated benchmark
- Baseline:
452d5bb86b45416c35891262f6a11dbd6a970117, using that checkout's native module dependencies. The baseline callcheck.go also matches the one in v0.8.1 byte-for-byte. - Environment: Go 1.27.1, macOS/arm64, Apple M1 Pro.
- One operation runs a complete callcheck pass over prebuilt IR. Parsing, type checking, IR construction and the tokenfile prerequisite are outside timing; per-pass filtering, traversal, rule dispatch and diagnostic reporting are inside.
- Seven inputs, each with either 1 or 16 rules. Nonempty inputs contain exactly 100 static calls, checked before timing. The mixed case contains 20 calls in each of five categories, including 20 matches. Each iteration asserts 0, 20 or 100 diagnostics as appropriate.
- The 16-rule case adds 15 distinct, nonmatching rule names. The generic receiver is an ordinary
Box[T any]; the same-name miss uses a different receiver type. - Six samples per variant, alternating execution order,
-test.cpu=1 -test.benchtime=300ms, compared with benchstat. An earlier default-GOMAXPROCS run had substantial timing noise; the results below use the single-threaded rerun. This is a microbenchmark control, not a recommendation to change gopls settings. Some timing variability remains, as shown in the full output.
Selected results for one rule (100 calls per operation, except the empty input):
| Input | Baseline time/op | Experimental time/op | Time comparison |
|---|---|---|---|
| Function with a different name | 4.815µs | 1.532µs | −68.19%, p=.002 |
| Method with a different name | 31.146µs | 1.532µs | −95.08%, p=.002 |
| Generic method with a different name | 37.771µs | 1.536µs | −95.93%, p=.002 |
| Same method name, different receiver | 31.65µs | 32.75µs | +3.46%, p=.041 |
| All calls match | 56.23µs | 57.59µs | No significant difference detected, p=.240 |
| Mixed | 33.48µs | 19.82µs | −40.80%, p=.002 |
| No calls | 40.84ns | 90.48ns | +121.54%, p=.002 |
For the first three inputs, the one-rule experiment eliminates the measured allocations. The mixed input drops from approximately 31.27KiB to 18.29KiB per operation.
The filter has a cost. With 16 rules, building the set adds 936 bytes and 3 allocations per pass. The no-call case increases from 40.56ns to 841.8ns. For same-name misses and all-hit inputs, bytes/op increase by 2.85% and 1.54%, respectively. The mixed 16-rule case improves by 39.48% in time and about 38.6% in allocated bytes. These counterexamples are included because the proposed approach is not a win for every package or rule distribution. Lack of a significant timing difference is not proof of no regression.
These are callcheck pass measurements, not equivalent whole-program speedups or RSS reductions.
Full benchstat output (all 14 cases)
goos: darwin
goarch: arm64
pkg: honnef.co/go/tools/analysis/callcheck
cpu: Apple M1 Pro
│ before │ after │
│ sec/op │ sec/op vs base │
RuleMatching/rules1/empty 40.84n ± 8% 90.48n ± 3% +121.54% (p=0.002 n=6)
RuleMatching/rules1/function_miss 4.815µ ± 5% 1.532µ ± 21% -68.19% (p=0.002 n=6)
RuleMatching/rules1/method_miss 31.146µ ± 13% 1.532µ ± 4% -95.08% (p=0.002 n=6)
RuleMatching/rules1/generic_method_miss 37.771µ ± 7% 1.536µ ± 4% -95.93% (p=0.002 n=6)
RuleMatching/rules1/same_name_miss 31.65µ ± 11% 32.75µ ± 2% +3.46% (p=0.041 n=6)
RuleMatching/rules1/hit 56.23µ ± 8% 57.59µ ± 14% ~ (p=0.240 n=6)
RuleMatching/rules1/mixed 33.48µ ± 12% 19.82µ ± 2% -40.80% (p=0.002 n=6)
RuleMatching/rules16/empty 40.56n ± 1% 841.80n ± 6% +1975.70% (p=0.002 n=6)
RuleMatching/rules16/function_miss 4.530µ ± 6% 1.989µ ± 10% -56.09% (p=0.002 n=6)
RuleMatching/rules16/method_miss 31.202µ ± 12% 2.015µ ± 3% -93.54% (p=0.002 n=6)
RuleMatching/rules16/generic_method_miss 38.483µ ± 11% 2.015µ ± 4% -94.76% (p=0.002 n=6)
RuleMatching/rules16/same_name_miss 32.59µ ± 14% 34.42µ ± 10% ~ (p=0.310 n=6)
RuleMatching/rules16/hit 57.19µ ± 123% 59.23µ ± 19% ~ (p=0.699 n=6)
RuleMatching/rules16/mixed 34.27µ ± 21% 20.74µ ± 12% -39.48% (p=0.002 n=6)
geomean 10.49µ 4.806µ -54.17%
│ before │ after │
│ B/op │ B/op vs base │
RuleMatching/rules1/empty 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=6) ¹
RuleMatching/rules1/function_miss 2.344Ki ± 0% 0.000Ki ± 0% -100.00% (p=0.002 n=6)
RuleMatching/rules1/method_miss 30.47Ki ± 0% 0.00Ki ± 0% -100.00% (p=0.002 n=6)
RuleMatching/rules1/generic_method_miss 32.03Ki ± 0% 0.00Ki ± 0% -100.00% (p=0.002 n=6)
RuleMatching/rules1/same_name_miss 32.03Ki ± 0% 32.03Ki ± 0% ~ (p=1.000 n=6) ¹
RuleMatching/rules1/hit 59.38Ki ± 0% 59.38Ki ± 0% ~ (p=1.000 n=6) ¹
RuleMatching/rules1/mixed 31.25Ki ± 0% 18.28Ki ± 0% -41.50% (p=0.002 n=6)
RuleMatching/rules16/empty 0.0 ± 0% 936.0 ± 0% ? (p=0.002 n=6)
RuleMatching/rules16/function_miss 2400.0 ± 0% 936.0 ± 0% -61.00% (p=0.002 n=6)
RuleMatching/rules16/method_miss 31201.0 ± 0% 936.0 ± 0% -97.00% (p=0.002 n=6)
RuleMatching/rules16/generic_method_miss 32801.0 ± 0% 936.0 ± 0% -97.15% (p=0.002 n=6)
RuleMatching/rules16/same_name_miss 32.03Ki ± 0% 32.95Ki ± 0% +2.85% (p=0.002 n=6)
RuleMatching/rules16/hit 59.38Ki ± 0% 60.29Ki ± 0% +1.54% (p=0.002 n=6)
RuleMatching/rules16/mixed 31.25Ki ± 0% 19.20Ki ± 0% -38.58% (p=0.002 n=6)
geomean ² ? ²
¹ all samples are equal
² summaries must be >0 to compute geomean
│ before │ after │
│ allocs/op │ allocs/op vs base │
RuleMatching/rules1/empty 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=6) ¹
RuleMatching/rules1/function_miss 100.0 ± 0% 0.0 ± 0% -100.00% (p=0.002 n=6)
RuleMatching/rules1/method_miss 400.0 ± 0% 0.0 ± 0% -100.00% (p=0.002 n=6)
RuleMatching/rules1/generic_method_miss 400.0 ± 0% 0.0 ± 0% -100.00% (p=0.002 n=6)
RuleMatching/rules1/same_name_miss 400.0 ± 0% 400.0 ± 0% ~ (p=1.000 n=6) ¹
RuleMatching/rules1/hit 900.0 ± 0% 900.0 ± 0% ~ (p=1.000 n=6) ¹
RuleMatching/rules1/mixed 440.0 ± 0% 260.0 ± 0% -40.91% (p=0.002 n=6)
RuleMatching/rules16/empty 0.000 ± 0% 3.000 ± 0% ? (p=0.002 n=6)
RuleMatching/rules16/function_miss 100.000 ± 0% 3.000 ± 0% -97.00% (p=0.002 n=6)
RuleMatching/rules16/method_miss 400.000 ± 0% 3.000 ± 0% -99.25% (p=0.002 n=6)
RuleMatching/rules16/generic_method_miss 400.000 ± 0% 3.000 ± 0% -99.25% (p=0.002 n=6)
RuleMatching/rules16/same_name_miss 400.0 ± 0% 403.0 ± 0% +0.75% (p=0.002 n=6)
RuleMatching/rules16/hit 900.0 ± 0% 903.0 ± 0% +0.33% (p=0.002 n=6)
RuleMatching/rules16/mixed 440.0 ± 0% 263.0 ± 0% -40.23% (p=0.002 n=6)
geomean ² ? ²
¹ all samples are equal
² summaries must be >0 to compute geomean
Reproduction
Place this standalone file in analysis/callcheck/callcheck_bench_test.go in both checkouts. No Chord checkout or external fixture is required.
Self-contained benchmark source
package callcheck
import (
"fmt"
"go/ast"
"go/parser"
"go/token"
"go/types"
"strings"
"testing"
"golang.org/x/tools/go/analysis"
"honnef.co/go/tools/analysis/facts/tokenfile"
"honnef.co/go/tools/go/ir"
"honnef.co/go/tools/go/ir/irutil"
"honnef.co/go/tools/internal/passes/buildir"
)
// BenchmarkRuleMatching measures one callcheck pass over a prebuilt IR package.
// Each nonempty input has 100 calls. Parsing, type checking and IR construction
// are excluded; per-pass rule filtering and diagnostic reporting are included.
// The one- and sixteen-rule cases expose the cost of constructing the filter,
// including packages with no calls and workloads where every call matches.
func BenchmarkRuleMatching(b *testing.B) {
for _, ruleCount := range []int{1, 16} {
for _, tc := range []struct {
name, body string
want int
}{
{"empty", "", 0},
{"function_miss", strings.Repeat("Other(1);", 100), 0},
{"method_miss", strings.Repeat("v.Other(1);", 100), 0},
{"generic_method_miss", strings.Repeat("box.Other(1);", 100), 0},
{"same_name_miss", strings.Repeat("OtherTarget.Target(1);", 100), 0},
{"hit", strings.Repeat("v.Target(1);", 100), 100},
{"mixed", strings.Repeat("v.Other(1); box.Other(1); Other(1); OtherTarget.Target(1); v.Target(1);", 20), 20},
} {
b.Run(fmt.Sprintf("rules%d/%s", ruleCount, tc.name), func(b *testing.B) {
const srcPrefix = `package bench
func Other(int) {}
type Value struct{}
func (*Value) Other(int) {}
func (*Value) Target(int) {}
type OtherValue struct{}
func (*OtherValue) Target(int) {}
type Box[T any] struct{ Value T }
func (*Box[T]) Other(int) {}
var v Value
var OtherTarget OtherValue
var box Box[int]
func calls() {
`
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, "bench.go", srcPrefix+tc.body+"}", parser.SkipObjectResolution)
if err != nil {
b.Fatal(err)
}
pkg := types.NewPackage("example.com/bench", "bench")
irpkg, info, err := irutil.BuildPackage(&types.Config{}, fset, pkg, []*ast.File{f}, 0)
if err != nil {
b.Fatal(err)
}
calls := 0
for _, fn := range irpkg.Functions {
eachCall(fn, func(_ *ir.Function, _ ir.CallInstruction, _ *ir.Function) { calls++ })
}
wantCalls := 100
if tc.body == "" {
wantCalls = 0
}
if calls != wantCalls {
b.Fatalf("fixture contains %d calls, want %d", calls, wantCalls)
}
reports := 0
pass := &analysis.Pass{
Analyzer: &analysis.Analyzer{Name: "callcheckbench"},
Fset: fset, Files: []*ast.File{f}, Pkg: pkg, TypesInfo: info,
ResultOf: map[*analysis.Analyzer]any{buildir.Analyzer: &buildir.IR{Pkg: irpkg, SrcFuncs: irpkg.Functions}},
Report: func(analysis.Diagnostic) { reports++ },
}
fileMap, err := tokenfile.Analyzer.Run(pass)
if err != nil {
b.Fatal(err)
}
pass.ResultOf[tokenfile.Analyzer] = fileMap
rules := map[string]Check{
"(*example.com/bench.Value).Target": func(c *Call) {
c.Args[0].Invalid("benchmark diagnostic")
},
}
for i := 1; i < ruleCount; i++ {
rules[fmt.Sprintf("example.com/absent.Unused%d", i)] = func(*Call) { b.Fatal("unexpected rule match") }
}
run := Analyzer(rules)
b.ReportAllocs()
b.ResetTimer()
for b.Loop() {
reports = 0
if _, err := run(pass); err != nil {
b.Fatal(err)
}
if reports != tc.want {
b.Fatalf("got %d diagnostics, want %d", reports, tc.want)
}
}
})
}
}
}Copy the same benchmark file to both the baseline and experimental checkouts. In each checkout, build the test binary (use distinct absolute output paths):
env -u GOWORK go test -c -o /tmp/callcheck-before.test ./analysis/callcheck
# In the experimental checkout:
env -u GOWORK go test -c -o /tmp/callcheck-after.test ./analysis/callcheckFirst check every benchmark case with -test.benchtime=1x. Then run alternating samples:
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/callcheck-$variant.test" \
-test.run='^$' -test.bench='^BenchmarkRuleMatching$' \
-test.cpu=1 -test.benchtime=300ms -test.benchmem \
> "/tmp/callcheck-$round-$variant.txt"
done
done
cat /tmp/callcheck-[1-6]-before.txt > /tmp/callcheck-before.txt
cat /tmp/callcheck-[1-6]-after.txt > /tmp/callcheck-after.txt
benchstat /tmp/callcheck-before.txt /tmp/callcheck-after.txtThe experimental checkout should differ only by the production change above. Locally I built the baseline using a Go overlay that restored the original callcheck.go, with identical benchmark and regression-test sources in both builds.
Real editing workload: supporting context
With full Staticcheck and the same additional gopls analyzers enabled, I compared gopls built from golang/tools@7a070f4290003c7dbfc0676699ff326ba37da349, using Staticcheck v0.8.1, against the same build with only this callcheck change. The source snapshot was Chord at e8c62cf1213c07c771fed826a3bfb85128b63df4.
Four warm-cache pairs covered edits to internal/agent/agent_request.go and internal/agent/agent_request_test.go, with three unique unsaved comment edits per process and alternating before/after order. Cumulative sampled allocation per edit fell about 8–9%, and sampled process peak RSS fell about 15–20%. CPU time fell about 4–10% for test-file edits, but was essentially unchanged for ordinary source edits. Post-GC live heap did not consistently decrease. Final published diagnostics matched field-for-field in each pair.
These are small-sample, single-project observations, not general performance guarantees. gopls separates its disk cache by executable hash; the first experimental run had a cold cache and higher RSS, so that first pair was excluded from the warm-cache comparison. Desktop/background activity was not fully controlled. The standalone benchmark above is the independently reproducible evidence for this report; the Chord figures are only supporting context.
Validation and scope
The local production change passed the full short test suite on the stated Staticcheck baseline. A real-IR comparison against the original full-name lookup covered generic receivers, aliases, promoted methods, method expressions/values, go/defer calls, and rule changes between invocations; its race run passed. The benchmark cases passed on both variants, including call-count and diagnostic-count assertions. The final callcheck tests and benchmark smoke run passed with -race, and go vet ./analysis/callcheck passed.
Would rejecting impossible short names be useful here, or would a different way of avoiding repeated full-name construction better fit callcheck's design? The per-pass set overhead above is a tradeoff to consider, rather than an assumption that this particular implementation is the preferred solution.
Activity