Skip to content

Commit 26fc662

Browse files
committed
perf(build): gate reserved-device-name check's ToUpper by segment length
hasReservedDeviceName called strings.ToUpper on every path segment before the reservedDeviceNames set lookup. strings.ToUpper is a no-op copy only when the input already matches the target case, so a typical (lowercase) segment always paid the allocation. Every reserved name is 3-4 bytes (CON, PRN, COM1-9, LPT1-9), so segments outside that length range can never match; gating the ToUpper call behind a length check skips the allocation entirely for the common case. Per docs/development/high-performance-go.md's "Compile regexes at package scope" sibling guidance on cheap pre-checks gating expensive work. Measured on a 3-path, 7-segment fixture: 602 ns/op & 10 allocs/op -> ~380 ns/op & 5 allocs/op (the two segments that do fall in the 3-4 byte range still allocate, as they must to compare). TestHasReservedDeviceName_AllocBudget pins the allocation ceiling, confirmed red against the pre-fix code and green after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016wWQmrtb8EMbn1hDgkESqE
1 parent 682f8b0 commit 26fc662

2 files changed

Lines changed: 89 additions & 0 deletions

File tree

internal/rules/build/alloc_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package build
2+
3+
import "testing"
4+
5+
// hasReservedDeviceNameBudget pins docs/development/high-performance-go.md's
6+
// "compile regexes at package scope" sibling pattern for case folding: gate
7+
// the strings.ToUpper allocation behind a cheap length check instead of
8+
// converting every path segment. reservedDeviceNames only holds 3-4 byte
9+
// entries (CON, PRN, COM1..9, LPT1..9), so a segment outside that length
10+
// range can never match and must not pay for the case-fold copy. Measured
11+
// baseline on a 3-path, 7-segment fixture: 10 allocs/op before the length
12+
// gate, 5 after (the two segments that do fall in the 3-4 byte range still
13+
// allocate, which is correct — only the always-allocate-on-every-segment
14+
// behavior regressed).
15+
const hasReservedDeviceNameAllocBudget = 5
16+
17+
// hasReservedDeviceNameFixturePaths mirrors a small set of real repo-style
18+
// paths: a mix of segment lengths, most outside the reserved-name range.
19+
var hasReservedDeviceNameFixturePaths = []string{
20+
"docs/readme.md",
21+
"internal/rules/foo.go",
22+
"scripts/build.sh",
23+
}
24+
25+
// TestHasReservedDeviceName_AllocBudget pins the allocation regression gate
26+
// under a normal `go test` run (not only `-bench`), matching the project's
27+
// paragraphstructure.TestCheckAllocBudget convention.
28+
func TestHasReservedDeviceName_AllocBudget(t *testing.T) {
29+
if testing.Short() {
30+
t.Skip("alloc gate skipped in -short mode")
31+
}
32+
33+
allocs := testing.AllocsPerRun(200, func() {
34+
for _, p := range hasReservedDeviceNameFixturePaths {
35+
_ = hasReservedDeviceName(p)
36+
}
37+
})
38+
t.Logf("hasReservedDeviceName allocs/op over %d paths = %.1f (budget = %d)",
39+
len(hasReservedDeviceNameFixturePaths), allocs, hasReservedDeviceNameAllocBudget)
40+
if allocs > float64(hasReservedDeviceNameAllocBudget) {
41+
t.Fatalf("hasReservedDeviceName allocs/op = %.1f, budget = %d; "+
42+
"the length gate before strings.ToUpper may have regressed",
43+
allocs, hasReservedDeviceNameAllocBudget)
44+
}
45+
}
46+
47+
// TestHasReservedDeviceName_Correctness pins that the length gate does not
48+
// change which paths are flagged: reserved names at every valid length
49+
// (3 and 4 bytes) still match, in any case, and non-reserved segments of
50+
// any length (including 3-4 byte look-alikes) do not.
51+
func TestHasReservedDeviceName_Correctness(t *testing.T) {
52+
cases := []struct {
53+
path string
54+
want bool
55+
}{
56+
{"CON", true},
57+
{"con", true},
58+
{"dir/NUL.txt", true},
59+
{"COM1.log", true},
60+
{"com9", true},
61+
{"LPT9", true},
62+
{"CONSOLE.md", false},
63+
{"docs/readme.md", false},
64+
{"foo", false},
65+
{"bar/baz.md", false},
66+
}
67+
for _, c := range cases {
68+
if got := hasReservedDeviceName(c.path); got != c.want {
69+
t.Errorf("hasReservedDeviceName(%q) = %v, want %v", c.path, got, c.want)
70+
}
71+
}
72+
}
73+
74+
// BenchmarkHasReservedDeviceName reports allocs/op alongside ns/op so a
75+
// regression shows up in `go test -bench` output too.
76+
func BenchmarkHasReservedDeviceName(b *testing.B) {
77+
b.ReportAllocs()
78+
for i := 0; i < b.N; i++ {
79+
for _, p := range hasReservedDeviceNameFixturePaths {
80+
_ = hasReservedDeviceName(p)
81+
}
82+
}
83+
}

internal/rules/build/rule.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,12 @@ func hasReservedDeviceName(p string) bool {
393393
if dot := strings.IndexByte(seg, '.'); dot >= 0 {
394394
seg = seg[:dot]
395395
}
396+
// Every reserved name is 3-4 bytes; gate the ToUpper allocation
397+
// behind a length check so an ordinary (usually longer) path
398+
// segment never pays for the case-fold copy.
399+
if len(seg) < 3 || len(seg) > 4 {
400+
continue
401+
}
396402
if setutil.Contains(reservedDeviceNames, strings.ToUpper(seg)) {
397403
return true
398404
}

0 commit comments

Comments
 (0)