Skip to content

Commit 98d48e8

Browse files
committed
perf(config): skip cloning the discarded side of a list-setting merge
mergeAny deep-cloned the earlier side of a list-typed settings leaf via toAnySlice before checking whether the rule's merge mode was Append or the default Replace, discarding that clone on the (common) Replace path. It then made a second independent copy of the later side, which toAnySlice had already cloned into a fresh backing array. Check the merge mode first, only clone earlier when Append actually needs it, and return the already-independent later clone directly on Replace. Ref docs/development/high-performance-go.md "Skip work you don't need". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y1J29TYsYdQrUeMxSAbox
1 parent d6a49a8 commit 98d48e8

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

internal/config/deepmerge.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,26 +55,49 @@ func mergeSettingsMap(ruleName string, earlier, later map[string]any) map[string
5555
// mergeAny merges later onto earlier for a single settings leaf or
5656
// nested value. Maps recurse, lists honor the rule's declared merge
5757
// mode, and everything else is replaced wholesale by later.
58+
//
59+
// The default merge mode (rule.MergeReplace) is by far the common
60+
// case — MergeAppend is an opt-in a rule declares via
61+
// rule.ListMerger. toAnySlice(earlier) fully deep-clones the earlier
62+
// side, so it is only called when settingMergeMode says the clone
63+
// will actually be used (the append branch); the replace branch
64+
// returns later's clone (ll) directly instead of copying it a second
65+
// time — toAnySlice already gives every case (`[]any`, `[]string`,
66+
// `[]int`) a fresh, independent backing array, so a second copy adds
67+
// nothing. See docs/development/high-performance-go.md "Skip work
68+
// you don't need".
5869
func mergeAny(ruleName, key string, earlier, later any) any {
5970
if em, ok := earlier.(map[string]any); ok {
6071
if lm, ok := later.(map[string]any); ok {
6172
return mergeSettingsMap(ruleName, em, lm)
6273
}
6374
}
64-
if el, ok := toAnySlice(earlier); ok {
75+
if isAnySliceType(earlier) {
6576
if ll, ok := toAnySlice(later); ok {
6677
if settingMergeMode(ruleName, key) == rule.MergeAppend {
78+
el, _ := toAnySlice(earlier)
6779
merged := make([]any, 0, len(el)+len(ll))
6880
merged = append(merged, el...)
6981
merged = append(merged, ll...)
7082
return merged
7183
}
72-
return append([]any(nil), ll...)
84+
return ll
7385
}
7486
}
7587
return cloneAny(later)
7688
}
7789

90+
// isAnySliceType reports whether v is one of the slice types
91+
// toAnySlice normalizes, without cloning it. Mirrors toAnySlice's
92+
// type switch exactly.
93+
func isAnySliceType(v any) bool {
94+
switch v.(type) {
95+
case []any, []string, []int:
96+
return true
97+
}
98+
return false
99+
}
100+
78101
// settingMergeMode returns the merge mode for a list-typed rule
79102
// setting, defaulting to rule.MergeReplace when the rule does not
80103
// implement rule.ListMerger or is not registered.

internal/config/deepmerge_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,29 @@ func TestMergeRuleCfgListReplacedByDefault(t *testing.T) {
6262
assert.Equal(t, []any{"c"}, got.Settings["exclude"], "list defaults to replace")
6363
}
6464

65+
// TestMergeAny_ListReplace_AllocBudget pins mergeAny's per-call
66+
// allocation count for the common list-replace path (a rule's list
67+
// setting with no ListMerger opt-in). mergeAny used to deep-clone the
68+
// discarded `earlier` side via toAnySlice even though the default
69+
// replace mode never uses it, and then made a second independent copy
70+
// of the already-independent `later` clone before returning it. See
71+
// docs/development/high-performance-go.md "Skip work you don't need".
72+
// This runs once per list-typed settings leaf present on both layers
73+
// being merged, on every config-signature cache miss.
74+
func TestMergeAny_ListReplace_AllocBudget(t *testing.T) {
75+
earlier := []any{"a", "b", "c"}
76+
later := []any{"d", "e"}
77+
allocs := testing.AllocsPerRun(200, func() {
78+
got := mergeAny("line-length", "exclude", earlier, later)
79+
if len(got.([]any)) != 2 {
80+
t.Fatalf("unexpected result: %v", got)
81+
}
82+
})
83+
if allocs > 3 {
84+
t.Fatalf("mergeAny (replace mode) allocs per call: want <= 3, got %v", allocs)
85+
}
86+
}
87+
6588
func TestMergeRuleCfgListAppendedWhenRuleOptsIn(t *testing.T) {
6689
earlier := RuleCfg{
6790
Enabled: true,

0 commit comments

Comments
 (0)