From cbdd043cc0d59a049479a8c19a68b0811b83b026 Mon Sep 17 00:00:00 2001 From: quobix Date: Tue, 20 Oct 2026 10:23:00 -0400 Subject: [PATCH 1/5] perf: add owned node-line snapshots without materializing node maps --- datamodel/low/node_lines_snapshot_test.go | 128 ++++++++++++++++++++++ datamodel/low/node_map.go | 24 ++++ 2 files changed, 152 insertions(+) create mode 100644 datamodel/low/node_lines_snapshot_test.go diff --git a/datamodel/low/node_lines_snapshot_test.go b/datamodel/low/node_lines_snapshot_test.go new file mode 100644 index 00000000..188223be --- /dev/null +++ b/datamodel/low/node_lines_snapshot_test.go @@ -0,0 +1,128 @@ +package low + +import ( + "fmt" + "github.com/pb33f/go-yaml" + "reflect" + "slices" + "sync" + "testing" +) + +func TestNodeMapGetNodeLines(t *testing.T) { + cases := []struct { + name string + values map[int]any + }{ + {"nil nodes", nil}, {"empty", map[int]any{}}, {"single", map[int]any{4: &yaml.Node{}}}, + {"mixed", map[int]any{-2: &yaml.Node{}, 0: []*yaml.Node{{}, {}}, 7: (*yaml.Node)(nil), 9: []*yaml.Node(nil), 11: []*yaml.Node{}, 13: "ignored", 14: nil}}, + {"invalid only", map[int]any{5: "invalid", 9: 42}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + nm := &NodeMap{} + if tc.values != nil { + nm.Nodes = &NodeLines{} + for k, v := range tc.values { + nm.Nodes.Store(k, v) + } + } + got := nm.GetNodeLines() + want := []int{} + for k := range nm.GetNodes() { + want = append(want, k) + } + slices.Sort(got) + slices.Sort(want) + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %v want %v", got, want) + } + got[0] = 999 + fresh := nm.GetNodeLines() + slices.Sort(fresh) + if !reflect.DeepEqual(fresh, want) { + t.Fatalf("snapshot aliases storage: %v", fresh) + } + }) + } +} + +func TestNodeMapGetNodeLinesMutation(t *testing.T) { + nm := &NodeMap{Nodes: &NodeLines{}} + nm.Nodes.Store(2, "discard") + nm.AddNode(3, &yaml.Node{}) + nm.Nodes.Store(2, &yaml.Node{}) + nm.AddNode(3, &yaml.Node{}) + before := nm.GetNodeLines() + slices.Sort(before) + nm.Nodes.Store(2, "now invalid") + nm.Nodes.Store(7, (*yaml.Node)(nil)) + nm.AddNode(9, &yaml.Node{}) + after := nm.GetNodeLines() + slices.Sort(after) + if !reflect.DeepEqual(before, []int{2, 3}) || !reflect.DeepEqual(after, []int{3, 7, 9}) { + t.Fatalf("before %v after %v", before, after) + } +} + +func TestNodeMapGetNodeLinesConcurrent(t *testing.T) { + nm := &NodeMap{Nodes: &NodeLines{}} + var wg sync.WaitGroup + for worker := 0; worker < 4; worker++ { + wg.Add(1) + go func(worker int) { + defer wg.Done() + for i := 0; i < 1000; i++ { + nm.Nodes.Store(worker*1000+i, &yaml.Node{}) + lines := nm.GetNodeLines() + seen := make(map[int]bool, len(lines)) + for _, line := range lines { + if seen[line] { + t.Errorf("duplicate line %d", line) + } + seen[line] = true + } + } + }(worker) + } + wg.Wait() + if len(nm.GetNodeLines()) != 4000 { + t.Fatal("missing writes") + } +} + +func BenchmarkNodeMapLineNumbers(b *testing.B) { + for _, size := range []int{1, 32, 1024} { + b.Run(fmt.Sprint(size), func(b *testing.B) { + nm := &NodeMap{Nodes: &NodeLines{}} + for i := 0; i < size; i++ { + nm.Nodes.Store(i, &yaml.Node{}) + } + nm.GetNodes() + b.Run("map", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + n := 0 + for range nm.GetNodes() { + n++ + } + if n != size { + b.Fatal(n) + } + } + }) + b.Run("lines", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + n := 0 + for range nm.GetNodeLines() { + n++ + } + if n != size { + b.Fatal(n) + } + } + }) + }) + } +} diff --git a/datamodel/low/node_map.go b/datamodel/low/node_map.go index 46f599fe..b1ace9c8 100644 --- a/datamodel/low/node_map.go +++ b/datamodel/low/node_map.go @@ -158,6 +158,30 @@ func (nm *NodeMap) GetNodes() map[int][]*yaml.Node { return composed } +// GetNodeLines returns an owned snapshot of the line numbers in GetNodes, in +// unspecified order. Only *yaml.Node and []*yaml.Node values contribute lines, +// including typed nil values. If no such values exist, the result is []int{0}. +// Later writes to Nodes do not change the returned slice. +func (nm *NodeMap) GetNodeLines() []int { + if nm.Nodes == nil { + return []int{0} + } + nm.Nodes.mu.Lock() + defer nm.Nodes.mu.Unlock() + index := nm.Nodes.index() + lines := make([]int, 0, len(index)) + for line, value := range index { + switch value.(type) { + case *yaml.Node, []*yaml.Node: + lines = append(lines, line) + } + } + if len(lines) == 0 { + lines = append(lines, 0) + } + return lines +} + // ExtractNodes will iterate over a *yaml.Node and extract all nodes with a line number into a map func (nm *NodeMap) ExtractNodes(node *yaml.Node, recurse bool) { if node == nil { From 39f7691ac3819d3bb39aef44ec9ba3aab92eedab Mon Sep 17 00:00:00 2001 From: quobix Date: Tue, 20 Oct 2026 14:49:03 -0400 Subject: [PATCH 2/5] perf(index): avoid redundant origin hashes and leaf traversal scratch --- index/origin_identity_reference_test.go | 66 +++++++ index/origin_identity_test.go | 122 +++++++++++++ index/search_rolodex.go | 8 +- index/utility_methods.go | 13 +- ...tility_methods_hash_fast_benchmark_test.go | 93 ++++++++++ index/utility_methods_hash_fast_test.go | 169 ++++++++++++++++++ 6 files changed, 466 insertions(+), 5 deletions(-) create mode 100644 index/origin_identity_reference_test.go create mode 100644 index/origin_identity_test.go create mode 100644 index/utility_methods_hash_fast_benchmark_test.go create mode 100644 index/utility_methods_hash_fast_test.go diff --git a/index/origin_identity_reference_test.go b/index/origin_identity_reference_test.go new file mode 100644 index 00000000..c2c61bf6 --- /dev/null +++ b/index/origin_identity_reference_test.go @@ -0,0 +1,66 @@ +package index + +import "github.com/pb33f/go-yaml" + +// frozen origin lookup from libopenapi cbdd043c. +func (r *Rolodex) findNodeOriginWithValueReference(key, value, refNode *yaml.Node, refValue string) *NodeOrigin { + if key == nil { + return nil + } + keyOrigin := r.FindNodeOrigin(key) + var valueOrigin *NodeOrigin + var valueHash string + if value != nil { + if keyOrigin != nil && keyOrigin.AbsoluteLocation == r.GetRootIndex().specAbsolutePath { + valueOrigin = r.GetRootIndex().FindNodeOrigin(value) + valueHash = HashNode(value) + if refNode != nil && refValue != "" { + return keyOrigin + } + origin, done := checkOrigin(originCheck{ + valueOrigin: valueOrigin, + valueHash: valueHash, + keyOrigin: keyOrigin, + value: value, + rolodex: r, + ref: refValue, + refNode: refNode, + }) + if done { + return origin + } else { + return nil + } + } else { + // the value is not in the root index, so we need to search all indexes + for i := range r.indexes { + idx := r.indexes[i] + if keyOrigin == nil { + keyOrigin = idx.FindNodeOrigin(key) + } + n := idx.FindNodeOrigin(value) + if n != nil { + if refNode != nil && refValue != "" { + return keyOrigin + } + + valueHash = HashNode(value) + nHash := HashNode(n.Node) + + if valueHash == nHash { + if keyOrigin.AbsoluteLocation != n.AbsoluteLocation { + if refNode == nil && refValue == "" { + keyOrigin.AbsoluteLocationValue = n.AbsoluteLocation + keyOrigin.LineValue = n.Line + keyOrigin.ColumnValue = n.Column + keyOrigin.ValueNode = n.Node + } + } + return keyOrigin + } + } + } + } + } + return keyOrigin +} diff --git a/index/origin_identity_test.go b/index/origin_identity_test.go new file mode 100644 index 00000000..9cd3a80e --- /dev/null +++ b/index/origin_identity_test.go @@ -0,0 +1,122 @@ +package index + +import ( + "fmt" + "testing" + + "github.com/pb33f/go-yaml" +) + +func originIdentityIndex(path string, nodes ...*yaml.Node) *SpecIndex { + idx := &SpecIndex{specAbsolutePath: path} + for _, n := range nodes { + for len(idx.nodeLines) <= n.Line { + idx.nodeLines = append(idx.nodeLines, nil) + } + idx.nodeLines[n.Line] = append(idx.nodeLines[n.Line], nodeLineEntry{column: int32(n.Column), node: n}) + } + sortNodeLines(idx.nodeLines) + return idx +} + +func originIdentityValue(line int, children int) *yaml.Node { + n := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq", Line: line, Column: 2} + for i := 0; i < children; i++ { + n.Content = append(n.Content, &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: fmt.Sprint(i), Line: line + i + 1, Column: 4}) + } + return n +} + +func TestOriginIdentityEquivalent(t *testing.T) { + key := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "root", Line: 1, Column: 1} + value := originIdentityValue(2, 2) + extKey := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "external", Line: 20, Column: 1} + extValue := originIdentityValue(30, 2) + r := &Rolodex{rootIndex: originIdentityIndex("root.yaml", key, value), indexes: []*SpecIndex{originIdentityIndex("key.yaml", extKey), originIdentityIndex("value.yaml", extValue)}} + clone := *value + mismatch := *value + mismatch.Value = "different" + missing := &yaml.Node{Kind: yaml.ScalarNode, Value: "missing", Line: 99, Column: 99} + for _, tc := range []struct { + name string + key, value *yaml.Node + }{ + {"root pointer", key, value}, {"root clone", key, &clone}, {"different content", key, &mismatch}, + {"external pointer", extKey, extValue}, {"root to external", key, extValue}, + {"nil key", nil, value}, {"nil value", key, nil}, {"missing value", key, missing}, + {"missing key", missing, extValue}, + } { + for _, refNode := range []*yaml.Node{nil, key} { + for _, ref := range []string{"", "#/schemas/Item"} { + t.Run(fmt.Sprintf("%s/refNode=%t/ref=%t", tc.name, refNode != nil, ref != ""), func(t *testing.T) { + call := func(f func(*yaml.Node, *yaml.Node, *yaml.Node, string) *NodeOrigin) (out *NodeOrigin, panicked bool) { + defer func() { + if recover() != nil { + panicked = true + } + }() + out = f(tc.key, tc.value, refNode, ref) + return + } + ClearHashCache() + want, wp := call(r.findNodeOriginWithValueReference) + ClearHashCache() + got, gp := call(r.FindNodeOriginWithValue) + if gp != wp || (got == nil) != (want == nil) || (got != nil && *got != *want) { + t.Fatalf("got %+v panic=%v, want %+v panic=%v", got, gp, want, wp) + } + if tc.name == "external pointer" && refNode == nil && ref == "" { + if got.AbsoluteLocation != "key.yaml" || got.AbsoluteLocationValue != "value.yaml" || got.LineValue != 30 || got.ColumnValue != 2 || got.ValueNode != extValue { + t.Fatalf("external metadata lost: %+v", got) + } + } + }) + } + } + } +} + +func TestOriginIdentityPreservesHashCachePopulation(t *testing.T) { + for _, external := range []bool{false, true} { + for _, withRef := range []bool{false, true} { + t.Run(fmt.Sprintf("external=%t/ref=%t", external, withRef), func(t *testing.T) { + key := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key", Line: 1, Column: 1} + value := originIdentityValue(2, hashCacheThreshold) + r := &Rolodex{rootIndex: originIdentityIndex("root.yaml", key, value)} + if external { + r.rootIndex = originIdentityIndex("root.yaml") + r.indexes = []*SpecIndex{originIdentityIndex("key.yaml", key), originIdentityIndex("value.yaml", value)} + } + var refNode *yaml.Node + ref := "" + if withRef { + refNode = key + ref = "#/schemas/Item" + } + ClearHashCache() + if r.FindNodeOriginWithValue(key, value, refNode, ref) == nil { + t.Fatal("missing origin") + } + cached, ok := nodeHashCache.Load(value) + // The existing external-reference early return occurs before hashing the value. + if external && withRef { + if ok { + t.Fatal("external ref unexpectedly populated value cache") + } + return + } + if !ok { + t.Fatal("origin lookup no longer populates the value hash cache") + } + value.Content[0].Value = "changed" + if HashNode(value) != cached { + t.Fatal("cached hash changed without invalidation") + } + ClearHashCache() + if HashNode(value) == cached { + t.Fatal("cache invalidation ignored mutation") + } + }) + } + } +} diff --git a/index/search_rolodex.go b/index/search_rolodex.go index 2d88f9d6..d5ab137d 100644 --- a/index/search_rolodex.go +++ b/index/search_rolodex.go @@ -25,6 +25,10 @@ func (r *Rolodex) FindNodeOriginWithValue(key, value, refNode *yaml.Node, refVal if refNode != nil && refValue != "" { return keyOrigin } + // keep the first hash for cache population, but identical nodes need no second hash. + if valueOrigin != nil && valueOrigin.Node == value { + return keyOrigin + } origin, done := checkOrigin(originCheck{ valueOrigin: valueOrigin, valueHash: valueHash, @@ -53,9 +57,7 @@ func (r *Rolodex) FindNodeOriginWithValue(key, value, refNode *yaml.Node, refVal } valueHash = HashNode(value) - nHash := HashNode(n.Node) - - if valueHash == nHash { + if n.Node == value || valueHash == HashNode(n.Node) { if keyOrigin.AbsoluteLocation != n.AbsoluteLocation { if refNode == nil && refValue == "" { keyOrigin.AbsoluteLocationValue = n.AbsoluteLocation diff --git a/index/utility_methods.go b/index/utility_methods.go index addca04d..996c8db0 100644 --- a/index/utility_methods.go +++ b/index/utility_methods.go @@ -735,6 +735,16 @@ func HashNode(n *yaml.Node) string { h.Reset() defer hasherPool.Put(h) + // a leaf cannot revisit a node, so it needs no traversal storage. + // keep this after the cache lookup: callers invalidate cached nodes explicitly. + if len(n.Content) == 0 { + h.WriteString(n.Tag) + writeIntToHash(h, n.Line) + writeIntToHash(h, n.Column) + h.WriteString(n.Value) + return strconv.FormatUint(h.Sum64(), 16) + } + // get stack from pool, reset length but keep capacity stackPtr := stackPool.Get().(*[]*yaml.Node) stack := (*stackPtr)[:0] @@ -744,9 +754,8 @@ func HashNode(n *yaml.Node) string { stackPool.Put(stackPtr) }() - // get visited map from pool, clear entries + // every map is emptied before it is returned to this private pool. visited := visitedPool.Get().(map[*yaml.Node]struct{}) - clear(visited) defer func() { clear(visited) visitedPool.Put(visited) diff --git a/index/utility_methods_hash_fast_benchmark_test.go b/index/utility_methods_hash_fast_benchmark_test.go new file mode 100644 index 00000000..ccd79531 --- /dev/null +++ b/index/utility_methods_hash_fast_benchmark_test.go @@ -0,0 +1,93 @@ +package index + +import ( + "strconv" + "testing" + + "github.com/pb33f/go-yaml" +) + +var hashNodeFastBenchmarkSink string + +func hashNodeFastBenchmarkChain(size int) *yaml.Node { + root := &yaml.Node{Tag: "!!seq"} + current := root + for i := 1; i < size; i++ { + next := &yaml.Node{Tag: "!!seq", Value: strconv.Itoa(i), Line: i} + current.Content = []*yaml.Node{next} + current = next + } + return root +} + +// BenchmarkHashNodeTraversalWorkloads distinguishes scalar traffic, uncached +// narrow roots, cold wide roots, and map reuse after a large traversal. +func BenchmarkHashNodeTraversalWorkloads(b *testing.B) { + leaf := &yaml.Node{Tag: "!!str", Value: "property", Line: 13, Column: 5} + small := &yaml.Node{Tag: "!!map", Content: []*yaml.Node{{Value: "type"}, {Value: "string"}}} + narrow := hashNodeFastBenchmarkChain(16384) + wide := &yaml.Node{Tag: "!!seq", Content: make([]*yaml.Node, 16384)} + for i := range wide.Content { + wide.Content[i] = &yaml.Node{Value: strconv.Itoa(i), Line: i} + } + for _, workload := range []struct { + name string + root *yaml.Node + }{{"leaf", leaf}, {"small_composite", small}, {"large_narrow_uncached_repeat", narrow}} { + b.Run(workload.name, func(b *testing.B) { + ClearHashCache() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(workload.root) + } + b.StopTimer() + b.ReportMetric(1, "hashes/op") + }) + } + for _, workload := range []struct { + name string + small *yaml.Node + }{{"leaf_after_large_warmup", leaf}, {"composite_after_large_warmup", small}} { + b.Run(workload.name, func(b *testing.B) { + ClearHashCache() + HashNode(narrow) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(workload.small) + } + b.StopTimer() + b.ReportMetric(1, "hashes/op") + }) + } + b.Run("large_wide_cold_with_cache_clear", func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + ClearHashCache() + hashNodeFastBenchmarkSink = HashNode(wide) + } + b.StopTimer() + ClearHashCache() + b.ReportMetric(1, "hashes/op") + }) + for _, workload := range []struct { + name string + small *yaml.Node + }{{"large_then_128_leaves", leaf}, {"large_then_128_composites", small}} { + b.Run(workload.name, func(b *testing.B) { + ClearHashCache() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(narrow) + for j := 0; j < 128; j++ { + hashNodeFastBenchmarkSink = HashNode(workload.small) + } + } + b.StopTimer() + b.ReportMetric(129, "hashes/op") + }) + } +} diff --git a/index/utility_methods_hash_fast_test.go b/index/utility_methods_hash_fast_test.go new file mode 100644 index 00000000..adb4a6e8 --- /dev/null +++ b/index/utility_methods_hash_fast_test.go @@ -0,0 +1,169 @@ +package index + +import ( + "hash/maphash" + "math/rand" + "strconv" + "sync" + "testing" + + "github.com/pb33f/go-yaml" +) + +// frozenUncachedHashNode preserves the pre-optimization byte stream independently +// of the production writer, caches, and pools. Aliases are not traversed. +func frozenUncachedHashNode(root *yaml.Node) string { + var h maphash.Hash + h.SetSeed(globalHashSeed) + stack := []*yaml.Node{root} + seen := make(map[*yaml.Node]bool) + for len(stack) > 0 { + n := stack[len(stack)-1] + stack = stack[:len(stack)-1] + if n == nil || seen[n] { + continue + } + seen[n] = true + h.WriteString(n.Tag) + // The historical writer emits nothing for negative coordinates. + if n.Line >= 0 { + h.WriteString(strconv.Itoa(n.Line)) + } + if n.Column >= 0 { + h.WriteString(strconv.Itoa(n.Column)) + } + h.WriteString(n.Value) + for i := len(n.Content) - 1; i >= 0; i-- { + stack = append(stack, n.Content[i]) + } + } + return strconv.FormatUint(h.Sum64(), 16) +} + +func TestHashNode_FastPathFrozenOracle(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + leaf := &yaml.Node{Tag: "!!str", Value: "leaf", Line: 3, Column: 7} + cycle := &yaml.Node{Tag: "!!map", Line: -3, Column: -1} + cycle.Content = []*yaml.Node{nil, leaf, cycle, leaf} + alias := &yaml.Node{Kind: yaml.AliasNode, Value: "alias", Alias: cycle, Line: 9} + cases := []*yaml.Node{nil, {}, leaf, cycle, alias, {Content: []*yaml.Node{nil, nil}}, {Content: []*yaml.Node{leaf, alias, cycle}}} + rng := rand.New(rand.NewSource(424242)) + for iteration := 0; iteration < 400; iteration++ { + nodes := make([]*yaml.Node, 1+rng.Intn(80)) + for i := range nodes { + nodes[i] = &yaml.Node{Kind: yaml.Kind(1 << rng.Intn(5)), Tag: []string{"", "!!str", "!!map", "!!seq"}[rng.Intn(4)], Value: strconv.Itoa(rng.Intn(20)), Line: rng.Intn(200) - 10, Column: rng.Intn(30) - 5} + } + for _, n := range nodes { + n.Alias = nodes[rng.Intn(len(nodes))] + for j := rng.Intn(25); j > 0; j-- { + if rng.Intn(7) == 0 { + n.Content = append(n.Content, nil) + } else { + n.Content = append(n.Content, nodes[rng.Intn(len(nodes))]) + } + } + } + cases = append(cases, nodes[0]) + } + for i, n := range cases { + want := frozenUncachedHashNode(n) + if got := HashNode(n); got != want { + t.Fatalf("case %d: got %s want %s", i, got, want) + } + if got := HashNode(n); got != want { + t.Fatalf("repeat case %d: got %s want %s", i, got, want) + } + } +} + +func TestHashNode_FastPathMutationAndThreshold(t *testing.T) { + for _, size := range []int{hashCacheThreshold - 1, hashCacheThreshold, hashCacheThreshold + 1} { + t.Run(strconv.Itoa(size), func(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + root := &yaml.Node{Tag: "!!seq", Content: make([]*yaml.Node, size)} + for i := range root.Content { + root.Content[i] = &yaml.Node{Value: strconv.Itoa(i)} + } + original := HashNode(root) + _, cached := nodeHashCache.Load(root) + if cached != (size >= hashCacheThreshold) { + t.Fatalf("cache threshold changed at %d", size) + } + root.Content = nil + root.Value = "mutated to leaf" + want := frozenUncachedHashNode(root) + if cached { + want = original + } + if got := HashNode(root); got != want { + t.Fatalf("cached-large-to-leaf semantics changed: got %s want %s", got, want) + } + ClearHashCache() + if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { + t.Fatalf("clear did not expose mutation: got %s want %s", got, want) + } + root.Value = "mutated leaf again" + if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { + t.Fatalf("uncached leaf mutation: got %s want %s", got, want) + } + }) + } +} + +func TestHashNode_FastPathPoolsReleaseNodes(t *testing.T) { + root := &yaml.Node{Content: make([]*yaml.Node, 17)} + for i := range root.Content { + root.Content[i] = &yaml.Node{Content: []*yaml.Node{root, nil}, Value: strconv.Itoa(i)} + } + HashNode(root) + visited := visitedPool.Get().(map[*yaml.Node]struct{}) + if len(visited) != 0 { + t.Errorf("pooled visited map retains %d nodes", len(visited)) + } + visitedPool.Put(visited) + stack := stackPool.Get().(*[]*yaml.Node) + if len(*stack) != 0 { + t.Errorf("pooled stack length is %d", len(*stack)) + } + for i, n := range (*stack)[:cap(*stack)] { + if n != nil { + t.Errorf("pooled stack retains node at %d", i) + } + } + stackPool.Put(stack) +} + +func TestHashNode_FastPathConcurrent(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + leaf := &yaml.Node{Value: "leaf", Line: 8} + root := &yaml.Node{Content: make([]*yaml.Node, hashCacheThreshold)} + for i := range root.Content { + root.Content[i] = leaf + } + root.Content[0] = root + roots := []*yaml.Node{nil, leaf, root} + wants := make([]string, len(roots)) + for i, n := range roots { + wants[i] = frozenUncachedHashNode(n) + } + var wg sync.WaitGroup + for worker := 0; worker < 8; worker++ { + wg.Add(1) + go func() { + defer wg.Done() + for iteration := 0; iteration < 300; iteration++ { + for i, n := range roots { + if got := HashNode(n); got != wants[i] { + t.Errorf("concurrent mismatch: got %s want %s", got, wants[i]) + return + } + } + ClearNodePools() + } + }() + } + wg.Wait() +} From b02d232ca5f5c3928442c4a8acf71d571990f353 Mon Sep 17 00:00:00 2001 From: quobix Date: Tue, 20 Oct 2026 20:55:40 -0400 Subject: [PATCH 3/5] perf(index): keep small hash traversals in local buffers --- index/utility_methods.go | 104 +++++++++++++++++------ index/utility_methods_hash_small_test.go | 99 +++++++++++++++++++++ 2 files changed, 175 insertions(+), 28 deletions(-) create mode 100644 index/utility_methods_hash_small_test.go diff --git a/index/utility_methods.go b/index/utility_methods.go index 996c8db0..c82eec7e 100644 --- a/index/utility_methods.go +++ b/index/utility_methods.go @@ -745,48 +745,96 @@ func HashNode(n *yaml.Node) string { return strconv.FormatUint(h.Sum64(), 16) } - // get stack from pool, reset length but keep capacity - stackPtr := stackPool.Get().(*[]*yaml.Node) - stack := (*stackPtr)[:0] - stack = append(stack, n) + // small subtrees use local storage; pooled state is acquired only on overflow. + // keep pooledStack separate so the local array never escapes through the pool. + var localStack [16]*yaml.Node + localStack[0] = n + pending := 1 + var stackPtr *[]*yaml.Node + var pooledStack []*yaml.Node + var localVisited [16]*yaml.Node + visitedCount := 0 + var visited map[*yaml.Node]struct{} defer func() { - *stackPtr = stack[:0] - stackPool.Put(stackPtr) - }() - - // every map is emptied before it is returned to this private pool. - visited := visitedPool.Get().(map[*yaml.Node]struct{}) - defer func() { - clear(visited) - visitedPool.Put(visited) + if stackPtr != nil { + clear(pooledStack) + *stackPtr = pooledStack[:0] + stackPool.Put(stackPtr) + } + if visited != nil { + clear(visited) + visitedPool.Put(visited) + } }() - for len(stack) > 0 { - // pop from stack - // clear the slot so the pooled slice never keeps a popped node alive. - node := stack[len(stack)-1] - stack[len(stack)-1] = nil - stack = stack[:len(stack)-1] - + for pending > 0 || len(pooledStack) > 0 { + var node *yaml.Node + if stackPtr == nil { + pending-- + node = localStack[pending] + localStack[pending] = nil + } else { + last := len(pooledStack) - 1 + node = pooledStack[last] + pooledStack[last] = nil + pooledStack = pooledStack[:last] + } if node == nil { continue } - // skip already visited nodes (handles circular references) - if _, seen := visited[node]; seen { - continue + // visited-once semantics apply to shared children as well as cycles. + if visited == nil { + seen := false + for i := 0; i < visitedCount; i++ { + if localVisited[i] == node { + seen = true + break + } + } + if seen { + continue + } + if visitedCount < len(localVisited) { + localVisited[visitedCount] = node + visitedCount++ + } else { + // every pooled map is emptied before it is returned. + visited = visitedPool.Get().(map[*yaml.Node]struct{}) + for _, previous := range localVisited { + visited[previous] = struct{}{} + } + clear(localVisited[:]) + visited[node] = struct{}{} + } + } else { + if _, seen := visited[node]; seen { + continue + } + visited[node] = struct{}{} } - visited[node] = struct{}{} - // hash node content - WriteString for strings, writeIntToHash for ints (zero allocations) h.WriteString(node.Tag) writeIntToHash(h, node.Line) writeIntToHash(h, node.Column) h.WriteString(node.Value) - // push children in reverse order for correct traversal order - for i := len(node.Content) - 1; i >= 0; i-- { - stack = append(stack, node.Content[i]) + // preserve pending entries and reverse child order when storage grows. + if stackPtr == nil && len(node.Content) > len(localStack)-pending { + stackPtr = stackPool.Get().(*[]*yaml.Node) + pooledStack = append((*stackPtr)[:0], localStack[:pending]...) + clear(localStack[:pending]) + pending = 0 + } + if stackPtr == nil { + for i := len(node.Content) - 1; i >= 0; i-- { + localStack[pending] = node.Content[i] + pending++ + } + } else { + for i := len(node.Content) - 1; i >= 0; i-- { + pooledStack = append(pooledStack, node.Content[i]) + } } } diff --git a/index/utility_methods_hash_small_test.go b/index/utility_methods_hash_small_test.go new file mode 100644 index 00000000..633c615a --- /dev/null +++ b/index/utility_methods_hash_small_test.go @@ -0,0 +1,99 @@ +package index + +import ( + "fmt" + "strconv" + "testing" + + "github.com/pb33f/go-yaml" +) + +func TestHashNode_SmallTraversalTransitions(t *testing.T) { + for _, size := range []int{8, 15, 16, 17, 18, 31, 32, 33} { + t.Run(strconv.Itoa(size), func(t *testing.T) { + leaf := &yaml.Node{Tag: "!!str", Value: "shared", Line: 4} + wideShared := &yaml.Node{Content: make([]*yaml.Node, size)} + wideUnique := &yaml.Node{Content: make([]*yaml.Node, size)} + for i := 0; i < size; i++ { + wideShared.Content[i] = leaf + wideUnique.Content[i] = &yaml.Node{Value: strconv.Itoa(i), Line: i, Column: i % 3} + } + // stack-only promotion includes nils and duplicates in pending capacity. + wideShared.Content[size/2] = nil + deep := hashNodeFastBenchmarkChain(size) + tail := deep + for len(tail.Content) > 0 { + tail = tail.Content[0] + } + // on size17 this ancestor cycle is reached directly after visited promotion. + tail.Content = []*yaml.Node{deep, leaf, leaf, nil} + // preserve existing pending siblings below newly pushed children on promotion. + pending := &yaml.Node{Content: []*yaml.Node{wideUnique, leaf, deep, nil, wideShared}} + cases := []struct { + name string + root *yaml.Node + }{{"stack_only", wideShared}, {"both", wideUnique}, {"visited_only_cycle", deep}, {"pending_siblings", pending}} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + want := frozenUncachedHashNode(tc.root) + for repeat := 0; repeat < 3; repeat++ { + if got := HashNode(tc.root); got != want { + t.Fatalf("repeat %d got %s want %s", repeat, got, want) + } + } + }) + } + }) + } +} + +func TestHashNode_SmallTraversalOrderAndSharedNodes(t *testing.T) { + for _, width := range []int{16, 17, 32} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + root := &yaml.Node{Tag: "root"} + shared := &yaml.Node{Value: "shared"} + for i := 0; i < width; i++ { + root.Content = append(root.Content, &yaml.Node{Value: strconv.Itoa(i), Content: []*yaml.Node{shared, nil, root}}) + } + if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { + t.Fatalf("got %s want %s", got, want) + } + root.Content[0], root.Content[len(root.Content)-1] = root.Content[len(root.Content)-1], root.Content[0] + ClearHashCache() + if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { + t.Fatalf("reordered got %s want %s", got, want) + } + }) + } +} + +func BenchmarkHashNodeSmallTraversalBoundaries(b *testing.B) { + for _, size := range []int{8, 16, 17, 32} { + b.Run(fmt.Sprintf("deep_unique_%d", size), func(b *testing.B) { + root := hashNodeFastBenchmarkChain(size) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(root) + } + }) + b.Run(fmt.Sprintf("wide_duplicates_%d", size), func(b *testing.B) { + shared := &yaml.Node{Value: "same"} + wide := &yaml.Node{Content: make([]*yaml.Node, size)} + for i := range wide.Content { + wide.Content[i] = shared + } + // the narrow root avoids cache admission and measures each traversal. + root := &yaml.Node{Content: []*yaml.Node{wide}} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(root) + } + }) + } +} From d1a8313bb1d4945d02ebf4aacb2315abdcaeb946 Mon Sep 17 00:00:00 2001 From: quobix Date: Wed, 21 Oct 2026 15:55:18 -0400 Subject: [PATCH 4/5] perf(index): skip hashes for small origin identity matches --- index/origin_identity_reference_test.go | 66 -------- index/origin_identity_test.go | 190 ++++++++++++++++-------- index/search_rolodex.go | 15 +- 3 files changed, 134 insertions(+), 137 deletions(-) delete mode 100644 index/origin_identity_reference_test.go diff --git a/index/origin_identity_reference_test.go b/index/origin_identity_reference_test.go deleted file mode 100644 index c2c61bf6..00000000 --- a/index/origin_identity_reference_test.go +++ /dev/null @@ -1,66 +0,0 @@ -package index - -import "github.com/pb33f/go-yaml" - -// frozen origin lookup from libopenapi cbdd043c. -func (r *Rolodex) findNodeOriginWithValueReference(key, value, refNode *yaml.Node, refValue string) *NodeOrigin { - if key == nil { - return nil - } - keyOrigin := r.FindNodeOrigin(key) - var valueOrigin *NodeOrigin - var valueHash string - if value != nil { - if keyOrigin != nil && keyOrigin.AbsoluteLocation == r.GetRootIndex().specAbsolutePath { - valueOrigin = r.GetRootIndex().FindNodeOrigin(value) - valueHash = HashNode(value) - if refNode != nil && refValue != "" { - return keyOrigin - } - origin, done := checkOrigin(originCheck{ - valueOrigin: valueOrigin, - valueHash: valueHash, - keyOrigin: keyOrigin, - value: value, - rolodex: r, - ref: refValue, - refNode: refNode, - }) - if done { - return origin - } else { - return nil - } - } else { - // the value is not in the root index, so we need to search all indexes - for i := range r.indexes { - idx := r.indexes[i] - if keyOrigin == nil { - keyOrigin = idx.FindNodeOrigin(key) - } - n := idx.FindNodeOrigin(value) - if n != nil { - if refNode != nil && refValue != "" { - return keyOrigin - } - - valueHash = HashNode(value) - nHash := HashNode(n.Node) - - if valueHash == nHash { - if keyOrigin.AbsoluteLocation != n.AbsoluteLocation { - if refNode == nil && refValue == "" { - keyOrigin.AbsoluteLocationValue = n.AbsoluteLocation - keyOrigin.LineValue = n.Line - keyOrigin.ColumnValue = n.Column - keyOrigin.ValueNode = n.Node - } - } - return keyOrigin - } - } - } - } - } - return keyOrigin -} diff --git a/index/origin_identity_test.go b/index/origin_identity_test.go index 9cd3a80e..74e14424 100644 --- a/index/origin_identity_test.go +++ b/index/origin_identity_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" ) func originIdentityIndex(path string, nodes ...*yaml.Node) *SpecIndex { @@ -32,89 +34,145 @@ func TestOriginIdentityEquivalent(t *testing.T) { value := originIdentityValue(2, 2) extKey := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "external", Line: 20, Column: 1} extValue := originIdentityValue(30, 2) - r := &Rolodex{rootIndex: originIdentityIndex("root.yaml", key, value), indexes: []*SpecIndex{originIdentityIndex("key.yaml", extKey), originIdentityIndex("value.yaml", extValue)}} + rootIndex := originIdentityIndex("root.yaml", key, value) + keyIndex := originIdentityIndex("key.yaml", extKey) + valueIndex := originIdentityIndex("value.yaml", extValue) + r := &Rolodex{rootIndex: rootIndex, indexes: []*SpecIndex{keyIndex, valueIndex}} + rootOrigin := &NodeOrigin{Node: key, Line: 1, Column: 1, AbsoluteLocation: "root.yaml", Index: rootIndex} + externalOrigin := &NodeOrigin{Node: extKey, Line: 20, Column: 1, AbsoluteLocation: "key.yaml", Index: keyIndex} + rootWithValue := *rootOrigin + rootWithValue.AbsoluteLocationValue = "value.yaml" + rootWithValue.LineValue, rootWithValue.ColumnValue, rootWithValue.ValueNode = 30, 2, extValue + externalWithValue := *externalOrigin + externalWithValue.AbsoluteLocationValue = "value.yaml" + externalWithValue.LineValue, externalWithValue.ColumnValue, externalWithValue.ValueNode = 30, 2, extValue clone := *value mismatch := *value mismatch.Value = "different" missing := &yaml.Node{Kind: yaml.ScalarNode, Value: "missing", Line: 99, Column: 99} - for _, tc := range []struct { + refs := []struct { + name string + node *yaml.Node + value string + }{ + {name: "none"}, + {name: "node only", node: key}, + {name: "value only", value: "#/schemas/Item"}, + {name: "both", node: key, value: "#/schemas/Item"}, + } + cases := []struct { name string key, value *yaml.Node + want [4]*NodeOrigin + panics [4]bool }{ - {"root pointer", key, value}, {"root clone", key, &clone}, {"different content", key, &mismatch}, - {"external pointer", extKey, extValue}, {"root to external", key, extValue}, - {"nil key", nil, value}, {"nil value", key, nil}, {"missing value", key, missing}, - {"missing key", missing, extValue}, - } { - for _, refNode := range []*yaml.Node{nil, key} { - for _, ref := range []string{"", "#/schemas/Item"} { - t.Run(fmt.Sprintf("%s/refNode=%t/ref=%t", tc.name, refNode != nil, ref != ""), func(t *testing.T) { - call := func(f func(*yaml.Node, *yaml.Node, *yaml.Node, string) *NodeOrigin) (out *NodeOrigin, panicked bool) { - defer func() { - if recover() != nil { - panicked = true - } - }() - out = f(tc.key, tc.value, refNode, ref) - return - } - ClearHashCache() - want, wp := call(r.findNodeOriginWithValueReference) - ClearHashCache() - got, gp := call(r.FindNodeOriginWithValue) - if gp != wp || (got == nil) != (want == nil) || (got != nil && *got != *want) { - t.Fatalf("got %+v panic=%v, want %+v panic=%v", got, gp, want, wp) - } - if tc.name == "external pointer" && refNode == nil && ref == "" { - if got.AbsoluteLocation != "key.yaml" || got.AbsoluteLocationValue != "value.yaml" || got.LineValue != 30 || got.ColumnValue != 2 || got.ValueNode != extValue { - t.Fatalf("external metadata lost: %+v", got) - } - } - }) - } + { + name: "root pointer", + key: key, + value: value, + want: [4]*NodeOrigin{rootOrigin, rootOrigin, rootOrigin, rootOrigin}, + }, + { + name: "root clone", + key: key, + value: &clone, + want: [4]*NodeOrigin{rootOrigin, rootOrigin, rootOrigin, rootOrigin}, + }, + { + name: "different content", + key: key, + value: &mismatch, + want: [4]*NodeOrigin{nil, nil, nil, rootOrigin}, + }, + { + name: "external pointer", + key: extKey, + value: extValue, + want: [4]*NodeOrigin{&externalWithValue, externalOrigin, externalOrigin, externalOrigin}, + }, + { + name: "root to external", + key: key, + value: extValue, + want: [4]*NodeOrigin{&rootWithValue, rootOrigin, rootOrigin, rootOrigin}, + }, + { + name: "nil key", + value: value, + }, + { + name: "nil value", + key: key, + want: [4]*NodeOrigin{rootOrigin, rootOrigin, rootOrigin, rootOrigin}, + }, + { + name: "missing value", + key: key, + value: missing, + want: [4]*NodeOrigin{nil, nil, nil, rootOrigin}, + }, + { + name: "missing key", + key: missing, + value: extValue, + panics: [4]bool{true, true, true, false}, + }, + } + for _, tc := range cases { + for i, ref := range refs { + t.Run(tc.name+"/"+ref.name, func(t *testing.T) { + ClearHashCache() + t.Cleanup(ClearHashCache) + call := func() { assert.Equal(t, tc.want[i], r.FindNodeOriginWithValue(tc.key, tc.value, ref.node, ref.value)) } + if tc.panics[i] { + assert.Panics(t, call) + return + } + assert.NotPanics(t, call) + }) } } } -func TestOriginIdentityPreservesHashCachePopulation(t *testing.T) { +func TestOriginIdentityPopulatesHashCache(t *testing.T) { + for _, external := range []bool{false, true} { + t.Run(fmt.Sprintf("external=%t", external), func(t *testing.T) { + key := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key", Line: 1, Column: 1} + value := originIdentityValue(2, hashCacheThreshold) + r := &Rolodex{rootIndex: originIdentityIndex("root.yaml", key, value)} + if external { + r.rootIndex = originIdentityIndex("root.yaml") + r.indexes = []*SpecIndex{originIdentityIndex("key.yaml", key), originIdentityIndex("value.yaml", value)} + } + ClearHashCache() + t.Cleanup(ClearHashCache) + require.NotNil(t, r.FindNodeOriginWithValue(key, value, nil, "")) + cached, ok := nodeHashCache.Load(value) + require.True(t, ok, "origin lookup should warm cache-eligible values") + value.Content[0].Value = "changed" + ClearHashCache() + assert.NotEqual(t, cached, HashNode(value), "invalidation must expose mutation") + }) + } +} + +func BenchmarkOriginIdentity(b *testing.B) { for _, external := range []bool{false, true} { - for _, withRef := range []bool{false, true} { - t.Run(fmt.Sprintf("external=%t/ref=%t", external, withRef), func(t *testing.T) { - key := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "key", Line: 1, Column: 1} - value := originIdentityValue(2, hashCacheThreshold) + for _, children := range []int{0, 2, hashCacheThreshold} { + b.Run(fmt.Sprintf("external=%t/children=%d", external, children), func(b *testing.B) { + key := &yaml.Node{Value: "key", Line: 1, Column: 1} + value := originIdentityValue(2, children) r := &Rolodex{rootIndex: originIdentityIndex("root.yaml", key, value)} if external { r.rootIndex = originIdentityIndex("root.yaml") - r.indexes = []*SpecIndex{originIdentityIndex("key.yaml", key), originIdentityIndex("value.yaml", value)} - } - var refNode *yaml.Node - ref := "" - if withRef { - refNode = key - ref = "#/schemas/Item" - } - ClearHashCache() - if r.FindNodeOriginWithValue(key, value, refNode, ref) == nil { - t.Fatal("missing origin") + r.indexes = []*SpecIndex{originIdentityIndex("external.yaml", key, value)} } - cached, ok := nodeHashCache.Load(value) - // The existing external-reference early return occurs before hashing the value. - if external && withRef { - if ok { - t.Fatal("external ref unexpectedly populated value cache") + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + if r.FindNodeOriginWithValue(key, value, nil, "") == nil { + b.Fatal("missing origin") } - return - } - if !ok { - t.Fatal("origin lookup no longer populates the value hash cache") - } - value.Content[0].Value = "changed" - if HashNode(value) != cached { - t.Fatal("cached hash changed without invalidation") - } - ClearHashCache() - if HashNode(value) == cached { - t.Fatal("cache invalidation ignored mutation") } }) } diff --git a/index/search_rolodex.go b/index/search_rolodex.go index d5ab137d..9de7a873 100644 --- a/index/search_rolodex.go +++ b/index/search_rolodex.go @@ -21,12 +21,15 @@ func (r *Rolodex) FindNodeOriginWithValue(key, value, refNode *yaml.Node, refVal if value != nil { if keyOrigin != nil && keyOrigin.AbsoluteLocation == r.GetRootIndex().specAbsolutePath { valueOrigin = r.GetRootIndex().FindNodeOrigin(value) - valueHash = HashNode(value) - if refNode != nil && refValue != "" { + if valueOrigin != nil && valueOrigin.Node == value { + // only cache-eligible nodes benefit from warming an identity match. + if len(value.Content) >= hashCacheThreshold { + HashNode(value) + } return keyOrigin } - // keep the first hash for cache population, but identical nodes need no second hash. - if valueOrigin != nil && valueOrigin.Node == value { + valueHash = HashNode(value) + if refNode != nil && refValue != "" { return keyOrigin } origin, done := checkOrigin(originCheck{ @@ -56,7 +59,9 @@ func (r *Rolodex) FindNodeOriginWithValue(key, value, refNode *yaml.Node, refVal return keyOrigin } - valueHash = HashNode(value) + if n.Node != value || len(value.Content) >= hashCacheThreshold { + valueHash = HashNode(value) + } if n.Node == value || valueHash == HashNode(n.Node) { if keyOrigin.AbsoluteLocation != n.AbsoluteLocation { if refNode == nil && refValue == "" { From f8bfdca105227413bbfc8469880a7a357058caec Mon Sep 17 00:00:00 2001 From: quobix Date: Wed, 21 Oct 2026 15:55:18 -0400 Subject: [PATCH 5/5] test: simplify hash coverage and clarify pool smoke checks --- datamodel/low/node_lines_snapshot_test.go | 47 ++-- ...=> utility_methods_hash_benchmark_test.go} | 39 ++- index/utility_methods_hash_fast_test.go | 169 ------------ index/utility_methods_hash_small_test.go | 99 ------- index/utility_methods_hash_test.go | 248 ++++++++++++++++++ 5 files changed, 303 insertions(+), 299 deletions(-) rename index/{utility_methods_hash_fast_benchmark_test.go => utility_methods_hash_benchmark_test.go} (73%) delete mode 100644 index/utility_methods_hash_fast_test.go delete mode 100644 index/utility_methods_hash_small_test.go create mode 100644 index/utility_methods_hash_test.go diff --git a/datamodel/low/node_lines_snapshot_test.go b/datamodel/low/node_lines_snapshot_test.go index 188223be..a68f72ff 100644 --- a/datamodel/low/node_lines_snapshot_test.go +++ b/datamodel/low/node_lines_snapshot_test.go @@ -2,11 +2,13 @@ package low import ( "fmt" - "github.com/pb33f/go-yaml" - "reflect" "slices" "sync" "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" ) func TestNodeMapGetNodeLines(t *testing.T) { @@ -14,9 +16,22 @@ func TestNodeMapGetNodeLines(t *testing.T) { name string values map[int]any }{ - {"nil nodes", nil}, {"empty", map[int]any{}}, {"single", map[int]any{4: &yaml.Node{}}}, - {"mixed", map[int]any{-2: &yaml.Node{}, 0: []*yaml.Node{{}, {}}, 7: (*yaml.Node)(nil), 9: []*yaml.Node(nil), 11: []*yaml.Node{}, 13: "ignored", 14: nil}}, - {"invalid only", map[int]any{5: "invalid", 9: 42}}, + {name: "nil nodes"}, + {name: "empty", values: map[int]any{}}, + {name: "single", values: map[int]any{4: &yaml.Node{}}}, + { + name: "mixed", + values: map[int]any{ + -2: &yaml.Node{}, + 0: []*yaml.Node{{}, {}}, + 7: (*yaml.Node)(nil), + 9: []*yaml.Node(nil), + 11: []*yaml.Node{}, + 13: "ignored", + 14: nil, + }, + }, + {name: "invalid only", values: map[int]any{5: "invalid", 9: 42}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -34,15 +49,12 @@ func TestNodeMapGetNodeLines(t *testing.T) { } slices.Sort(got) slices.Sort(want) - if !reflect.DeepEqual(got, want) { - t.Fatalf("got %v want %v", got, want) - } + assert.Equal(t, want, got) + require.NotEmpty(t, got) got[0] = 999 fresh := nm.GetNodeLines() slices.Sort(fresh) - if !reflect.DeepEqual(fresh, want) { - t.Fatalf("snapshot aliases storage: %v", fresh) - } + assert.Equal(t, want, fresh, "snapshot must not alias storage") }) } } @@ -60,9 +72,8 @@ func TestNodeMapGetNodeLinesMutation(t *testing.T) { nm.AddNode(9, &yaml.Node{}) after := nm.GetNodeLines() slices.Sort(after) - if !reflect.DeepEqual(before, []int{2, 3}) || !reflect.DeepEqual(after, []int{3, 7, 9}) { - t.Fatalf("before %v after %v", before, after) - } + assert.Equal(t, []int{2, 3}, before) + assert.Equal(t, []int{3, 7, 9}, after) } func TestNodeMapGetNodeLinesConcurrent(t *testing.T) { @@ -77,18 +88,14 @@ func TestNodeMapGetNodeLinesConcurrent(t *testing.T) { lines := nm.GetNodeLines() seen := make(map[int]bool, len(lines)) for _, line := range lines { - if seen[line] { - t.Errorf("duplicate line %d", line) - } + assert.False(t, seen[line], "duplicate line %d", line) seen[line] = true } } }(worker) } wg.Wait() - if len(nm.GetNodeLines()) != 4000 { - t.Fatal("missing writes") - } + assert.Len(t, nm.GetNodeLines(), 4000, "missing writes") } func BenchmarkNodeMapLineNumbers(b *testing.B) { diff --git a/index/utility_methods_hash_fast_benchmark_test.go b/index/utility_methods_hash_benchmark_test.go similarity index 73% rename from index/utility_methods_hash_fast_benchmark_test.go rename to index/utility_methods_hash_benchmark_test.go index ccd79531..85b5f570 100644 --- a/index/utility_methods_hash_fast_benchmark_test.go +++ b/index/utility_methods_hash_benchmark_test.go @@ -1,6 +1,7 @@ package index import ( + "fmt" "strconv" "testing" @@ -9,17 +10,6 @@ import ( var hashNodeFastBenchmarkSink string -func hashNodeFastBenchmarkChain(size int) *yaml.Node { - root := &yaml.Node{Tag: "!!seq"} - current := root - for i := 1; i < size; i++ { - next := &yaml.Node{Tag: "!!seq", Value: strconv.Itoa(i), Line: i} - current.Content = []*yaml.Node{next} - current = next - } - return root -} - // BenchmarkHashNodeTraversalWorkloads distinguishes scalar traffic, uncached // narrow roots, cold wide roots, and map reuse after a large traversal. func BenchmarkHashNodeTraversalWorkloads(b *testing.B) { @@ -91,3 +81,30 @@ func BenchmarkHashNodeTraversalWorkloads(b *testing.B) { }) } } + +func BenchmarkHashNodeSmallTraversalBoundaries(b *testing.B) { + for _, size := range []int{8, 16, 17, 32} { + b.Run(fmt.Sprintf("deep_unique_%d", size), func(b *testing.B) { + root := hashNodeFastBenchmarkChain(size) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(root) + } + }) + b.Run(fmt.Sprintf("wide_duplicates_%d", size), func(b *testing.B) { + shared := &yaml.Node{Value: "same"} + wide := &yaml.Node{Content: make([]*yaml.Node, size)} + for i := range wide.Content { + wide.Content[i] = shared + } + // the narrow root avoids cache admission and measures each traversal. + root := &yaml.Node{Content: []*yaml.Node{wide}} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + hashNodeFastBenchmarkSink = HashNode(root) + } + }) + } +} diff --git a/index/utility_methods_hash_fast_test.go b/index/utility_methods_hash_fast_test.go deleted file mode 100644 index adb4a6e8..00000000 --- a/index/utility_methods_hash_fast_test.go +++ /dev/null @@ -1,169 +0,0 @@ -package index - -import ( - "hash/maphash" - "math/rand" - "strconv" - "sync" - "testing" - - "github.com/pb33f/go-yaml" -) - -// frozenUncachedHashNode preserves the pre-optimization byte stream independently -// of the production writer, caches, and pools. Aliases are not traversed. -func frozenUncachedHashNode(root *yaml.Node) string { - var h maphash.Hash - h.SetSeed(globalHashSeed) - stack := []*yaml.Node{root} - seen := make(map[*yaml.Node]bool) - for len(stack) > 0 { - n := stack[len(stack)-1] - stack = stack[:len(stack)-1] - if n == nil || seen[n] { - continue - } - seen[n] = true - h.WriteString(n.Tag) - // The historical writer emits nothing for negative coordinates. - if n.Line >= 0 { - h.WriteString(strconv.Itoa(n.Line)) - } - if n.Column >= 0 { - h.WriteString(strconv.Itoa(n.Column)) - } - h.WriteString(n.Value) - for i := len(n.Content) - 1; i >= 0; i-- { - stack = append(stack, n.Content[i]) - } - } - return strconv.FormatUint(h.Sum64(), 16) -} - -func TestHashNode_FastPathFrozenOracle(t *testing.T) { - ClearHashCache() - defer ClearHashCache() - leaf := &yaml.Node{Tag: "!!str", Value: "leaf", Line: 3, Column: 7} - cycle := &yaml.Node{Tag: "!!map", Line: -3, Column: -1} - cycle.Content = []*yaml.Node{nil, leaf, cycle, leaf} - alias := &yaml.Node{Kind: yaml.AliasNode, Value: "alias", Alias: cycle, Line: 9} - cases := []*yaml.Node{nil, {}, leaf, cycle, alias, {Content: []*yaml.Node{nil, nil}}, {Content: []*yaml.Node{leaf, alias, cycle}}} - rng := rand.New(rand.NewSource(424242)) - for iteration := 0; iteration < 400; iteration++ { - nodes := make([]*yaml.Node, 1+rng.Intn(80)) - for i := range nodes { - nodes[i] = &yaml.Node{Kind: yaml.Kind(1 << rng.Intn(5)), Tag: []string{"", "!!str", "!!map", "!!seq"}[rng.Intn(4)], Value: strconv.Itoa(rng.Intn(20)), Line: rng.Intn(200) - 10, Column: rng.Intn(30) - 5} - } - for _, n := range nodes { - n.Alias = nodes[rng.Intn(len(nodes))] - for j := rng.Intn(25); j > 0; j-- { - if rng.Intn(7) == 0 { - n.Content = append(n.Content, nil) - } else { - n.Content = append(n.Content, nodes[rng.Intn(len(nodes))]) - } - } - } - cases = append(cases, nodes[0]) - } - for i, n := range cases { - want := frozenUncachedHashNode(n) - if got := HashNode(n); got != want { - t.Fatalf("case %d: got %s want %s", i, got, want) - } - if got := HashNode(n); got != want { - t.Fatalf("repeat case %d: got %s want %s", i, got, want) - } - } -} - -func TestHashNode_FastPathMutationAndThreshold(t *testing.T) { - for _, size := range []int{hashCacheThreshold - 1, hashCacheThreshold, hashCacheThreshold + 1} { - t.Run(strconv.Itoa(size), func(t *testing.T) { - ClearHashCache() - defer ClearHashCache() - root := &yaml.Node{Tag: "!!seq", Content: make([]*yaml.Node, size)} - for i := range root.Content { - root.Content[i] = &yaml.Node{Value: strconv.Itoa(i)} - } - original := HashNode(root) - _, cached := nodeHashCache.Load(root) - if cached != (size >= hashCacheThreshold) { - t.Fatalf("cache threshold changed at %d", size) - } - root.Content = nil - root.Value = "mutated to leaf" - want := frozenUncachedHashNode(root) - if cached { - want = original - } - if got := HashNode(root); got != want { - t.Fatalf("cached-large-to-leaf semantics changed: got %s want %s", got, want) - } - ClearHashCache() - if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { - t.Fatalf("clear did not expose mutation: got %s want %s", got, want) - } - root.Value = "mutated leaf again" - if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { - t.Fatalf("uncached leaf mutation: got %s want %s", got, want) - } - }) - } -} - -func TestHashNode_FastPathPoolsReleaseNodes(t *testing.T) { - root := &yaml.Node{Content: make([]*yaml.Node, 17)} - for i := range root.Content { - root.Content[i] = &yaml.Node{Content: []*yaml.Node{root, nil}, Value: strconv.Itoa(i)} - } - HashNode(root) - visited := visitedPool.Get().(map[*yaml.Node]struct{}) - if len(visited) != 0 { - t.Errorf("pooled visited map retains %d nodes", len(visited)) - } - visitedPool.Put(visited) - stack := stackPool.Get().(*[]*yaml.Node) - if len(*stack) != 0 { - t.Errorf("pooled stack length is %d", len(*stack)) - } - for i, n := range (*stack)[:cap(*stack)] { - if n != nil { - t.Errorf("pooled stack retains node at %d", i) - } - } - stackPool.Put(stack) -} - -func TestHashNode_FastPathConcurrent(t *testing.T) { - ClearHashCache() - defer ClearHashCache() - leaf := &yaml.Node{Value: "leaf", Line: 8} - root := &yaml.Node{Content: make([]*yaml.Node, hashCacheThreshold)} - for i := range root.Content { - root.Content[i] = leaf - } - root.Content[0] = root - roots := []*yaml.Node{nil, leaf, root} - wants := make([]string, len(roots)) - for i, n := range roots { - wants[i] = frozenUncachedHashNode(n) - } - var wg sync.WaitGroup - for worker := 0; worker < 8; worker++ { - wg.Add(1) - go func() { - defer wg.Done() - for iteration := 0; iteration < 300; iteration++ { - for i, n := range roots { - if got := HashNode(n); got != wants[i] { - t.Errorf("concurrent mismatch: got %s want %s", got, wants[i]) - return - } - } - ClearNodePools() - } - }() - } - wg.Wait() -} diff --git a/index/utility_methods_hash_small_test.go b/index/utility_methods_hash_small_test.go deleted file mode 100644 index 633c615a..00000000 --- a/index/utility_methods_hash_small_test.go +++ /dev/null @@ -1,99 +0,0 @@ -package index - -import ( - "fmt" - "strconv" - "testing" - - "github.com/pb33f/go-yaml" -) - -func TestHashNode_SmallTraversalTransitions(t *testing.T) { - for _, size := range []int{8, 15, 16, 17, 18, 31, 32, 33} { - t.Run(strconv.Itoa(size), func(t *testing.T) { - leaf := &yaml.Node{Tag: "!!str", Value: "shared", Line: 4} - wideShared := &yaml.Node{Content: make([]*yaml.Node, size)} - wideUnique := &yaml.Node{Content: make([]*yaml.Node, size)} - for i := 0; i < size; i++ { - wideShared.Content[i] = leaf - wideUnique.Content[i] = &yaml.Node{Value: strconv.Itoa(i), Line: i, Column: i % 3} - } - // stack-only promotion includes nils and duplicates in pending capacity. - wideShared.Content[size/2] = nil - deep := hashNodeFastBenchmarkChain(size) - tail := deep - for len(tail.Content) > 0 { - tail = tail.Content[0] - } - // on size17 this ancestor cycle is reached directly after visited promotion. - tail.Content = []*yaml.Node{deep, leaf, leaf, nil} - // preserve existing pending siblings below newly pushed children on promotion. - pending := &yaml.Node{Content: []*yaml.Node{wideUnique, leaf, deep, nil, wideShared}} - cases := []struct { - name string - root *yaml.Node - }{{"stack_only", wideShared}, {"both", wideUnique}, {"visited_only_cycle", deep}, {"pending_siblings", pending}} - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - ClearHashCache() - defer ClearHashCache() - want := frozenUncachedHashNode(tc.root) - for repeat := 0; repeat < 3; repeat++ { - if got := HashNode(tc.root); got != want { - t.Fatalf("repeat %d got %s want %s", repeat, got, want) - } - } - }) - } - }) - } -} - -func TestHashNode_SmallTraversalOrderAndSharedNodes(t *testing.T) { - for _, width := range []int{16, 17, 32} { - t.Run(fmt.Sprint(width), func(t *testing.T) { - ClearHashCache() - defer ClearHashCache() - root := &yaml.Node{Tag: "root"} - shared := &yaml.Node{Value: "shared"} - for i := 0; i < width; i++ { - root.Content = append(root.Content, &yaml.Node{Value: strconv.Itoa(i), Content: []*yaml.Node{shared, nil, root}}) - } - if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { - t.Fatalf("got %s want %s", got, want) - } - root.Content[0], root.Content[len(root.Content)-1] = root.Content[len(root.Content)-1], root.Content[0] - ClearHashCache() - if got, want := HashNode(root), frozenUncachedHashNode(root); got != want { - t.Fatalf("reordered got %s want %s", got, want) - } - }) - } -} - -func BenchmarkHashNodeSmallTraversalBoundaries(b *testing.B) { - for _, size := range []int{8, 16, 17, 32} { - b.Run(fmt.Sprintf("deep_unique_%d", size), func(b *testing.B) { - root := hashNodeFastBenchmarkChain(size) - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - hashNodeFastBenchmarkSink = HashNode(root) - } - }) - b.Run(fmt.Sprintf("wide_duplicates_%d", size), func(b *testing.B) { - shared := &yaml.Node{Value: "same"} - wide := &yaml.Node{Content: make([]*yaml.Node, size)} - for i := range wide.Content { - wide.Content[i] = shared - } - // the narrow root avoids cache admission and measures each traversal. - root := &yaml.Node{Content: []*yaml.Node{wide}} - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - hashNodeFastBenchmarkSink = HashNode(root) - } - }) - } -} diff --git a/index/utility_methods_hash_test.go b/index/utility_methods_hash_test.go new file mode 100644 index 00000000..94c77118 --- /dev/null +++ b/index/utility_methods_hash_test.go @@ -0,0 +1,248 @@ +package index + +import ( + "fmt" + "hash/maphash" + "math/rand" + "strconv" + "sync" + "testing" + + "github.com/pb33f/go-yaml" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" +) + +// frozenUncachedHashNode preserves the pre-optimization byte stream independently +// of the production writer, caches, and pools. Aliases are not traversed. +func frozenUncachedHashNode(root *yaml.Node) string { + var h maphash.Hash + h.SetSeed(globalHashSeed) + stack := []*yaml.Node{root} + seen := make(map[*yaml.Node]bool) + for len(stack) > 0 { + n := stack[len(stack)-1] + stack = stack[:len(stack)-1] + if n == nil || seen[n] { + continue + } + seen[n] = true + h.WriteString(n.Tag) + // The historical writer emits nothing for negative coordinates. + if n.Line >= 0 { + h.WriteString(strconv.Itoa(n.Line)) + } + if n.Column >= 0 { + h.WriteString(strconv.Itoa(n.Column)) + } + h.WriteString(n.Value) + for i := len(n.Content) - 1; i >= 0; i-- { + stack = append(stack, n.Content[i]) + } + } + return strconv.FormatUint(h.Sum64(), 16) +} + +func TestHashNode_FastPathFrozenOracle(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + leaf := &yaml.Node{Tag: "!!str", Value: "leaf", Line: 3, Column: 7} + cycle := &yaml.Node{Tag: "!!map", Line: -3, Column: -1} + cycle.Content = []*yaml.Node{nil, leaf, cycle, leaf} + alias := &yaml.Node{Kind: yaml.AliasNode, Value: "alias", Alias: cycle, Line: 9} + cases := []struct { + name string + node *yaml.Node + }{ + {name: "nil"}, + {name: "empty", node: &yaml.Node{}}, + {name: "leaf", node: leaf}, + {name: "cycle", node: cycle}, + {name: "alias", node: alias}, + {name: "nil_children", node: &yaml.Node{Content: []*yaml.Node{nil, nil}}}, + {name: "shared_children", node: &yaml.Node{Content: []*yaml.Node{leaf, alias, cycle}}}, + } + rng := rand.New(rand.NewSource(424242)) + for iteration := 0; iteration < 400; iteration++ { + nodes := make([]*yaml.Node, 1+rng.Intn(80)) + for i := range nodes { + nodes[i] = &yaml.Node{ + Kind: yaml.Kind(1 << rng.Intn(5)), + Tag: []string{"", "!!str", "!!map", "!!seq"}[rng.Intn(4)], + Value: strconv.Itoa(rng.Intn(20)), + Line: rng.Intn(200) - 10, + Column: rng.Intn(30) - 5, + } + } + for _, n := range nodes { + n.Alias = nodes[rng.Intn(len(nodes))] + for j := rng.Intn(25); j > 0; j-- { + if rng.Intn(7) == 0 { + n.Content = append(n.Content, nil) + } else { + n.Content = append(n.Content, nodes[rng.Intn(len(nodes))]) + } + } + } + cases = append(cases, struct { + name string + node *yaml.Node + }{name: fmt.Sprintf("random_%03d", iteration), node: nodes[0]}) + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + want := frozenUncachedHashNode(tc.node) + require.Equal(t, want, HashNode(tc.node)) + require.Equal(t, want, HashNode(tc.node), "repeated lookup") + }) + } + +} + +func TestHashNode_FastPathMutationAndThreshold(t *testing.T) { + for _, size := range []int{hashCacheThreshold - 1, hashCacheThreshold, hashCacheThreshold + 1} { + t.Run(strconv.Itoa(size), func(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + root := &yaml.Node{Tag: "!!seq", Content: make([]*yaml.Node, size)} + for i := range root.Content { + root.Content[i] = &yaml.Node{Value: strconv.Itoa(i)} + } + original := HashNode(root) + require.Equal(t, original, HashNode(root), "unchanged node retains its hash") + _, cached := nodeHashCache.Load(root) + require.Equal(t, size >= hashCacheThreshold, cached, "cache admission threshold") + root.Content = nil + root.Value = "mutated to leaf" + ClearHashCache() + require.Equal(t, frozenUncachedHashNode(root), HashNode(root), "invalidation exposes mutation") + root.Value = "mutated leaf again" + require.Equal(t, frozenUncachedHashNode(root), HashNode(root), "uncached leaf mutation") + + }) + } +} + +func TestHashNode_FastPathConcurrent(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + leaf := &yaml.Node{Value: "leaf", Line: 8} + root := &yaml.Node{Content: make([]*yaml.Node, hashCacheThreshold)} + for i := range root.Content { + root.Content[i] = leaf + } + root.Content[0] = root + roots := []*yaml.Node{nil, leaf, root} + wants := make([]string, len(roots)) + for i, n := range roots { + wants[i] = frozenUncachedHashNode(n) + } + var wg sync.WaitGroup + for worker := 0; worker < 8; worker++ { + wg.Add(1) + go func() { + defer wg.Done() + for iteration := 0; iteration < 300; iteration++ { + for i, n := range roots { + if !assert.Equal(t, wants[i], HashNode(n), "concurrent lookup") { + return + } + } + } + }() + } + wg.Wait() +} + +func hashNodeFastBenchmarkChain(size int) *yaml.Node { + root := &yaml.Node{Tag: "!!seq"} + current := root + for i := 1; i < size; i++ { + next := &yaml.Node{Tag: "!!seq", Value: strconv.Itoa(i), Line: i} + current.Content = []*yaml.Node{next} + current = next + } + return root +} + +func TestHashNode_SmallTraversalTransitions(t *testing.T) { + for _, size := range []int{8, 15, 16, 17, 18, 31, 32, 33} { + t.Run(strconv.Itoa(size), func(t *testing.T) { + leaf := &yaml.Node{Tag: "!!str", Value: "shared", Line: 4} + wideShared := &yaml.Node{Content: make([]*yaml.Node, size)} + wideUnique := &yaml.Node{Content: make([]*yaml.Node, size)} + for i := 0; i < size; i++ { + wideShared.Content[i] = leaf + wideUnique.Content[i] = &yaml.Node{Value: strconv.Itoa(i), Line: i, Column: i % 3} + } + // stack-only promotion includes nils and duplicates in pending capacity. + wideShared.Content[size/2] = nil + deep := hashNodeFastBenchmarkChain(size) + tail := deep + for len(tail.Content) > 0 { + tail = tail.Content[0] + } + // on size17 this ancestor cycle is reached directly after visited promotion. + tail.Content = []*yaml.Node{deep, leaf, leaf, nil} + // preserve existing pending siblings below newly pushed children on promotion. + pending := &yaml.Node{Content: []*yaml.Node{wideUnique, leaf, deep, nil, wideShared}} + cases := []struct { + name string + root *yaml.Node + }{ + {name: "stack_only", root: wideShared}, + {name: "both", root: wideUnique}, + {name: "visited_only_cycle", root: deep}, + {name: "pending_siblings", root: pending}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + want := frozenUncachedHashNode(tc.root) + for repeat := 0; repeat < 3; repeat++ { + require.Equal(t, want, HashNode(tc.root), "repeat %d", repeat) + } + }) + } + }) + } +} + +func TestHashNode_SmallTraversalOrderAndSharedNodes(t *testing.T) { + for _, width := range []int{16, 17, 32} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + ClearHashCache() + defer ClearHashCache() + root := &yaml.Node{Tag: "root"} + shared := &yaml.Node{Value: "shared"} + for i := 0; i < width; i++ { + root.Content = append(root.Content, &yaml.Node{Value: strconv.Itoa(i), Content: []*yaml.Node{shared, nil, root}}) + } + require.Equal(t, frozenUncachedHashNode(root), HashNode(root)) + root.Content[0], root.Content[len(root.Content)-1] = root.Content[len(root.Content)-1], root.Content[0] + ClearHashCache() + require.Equal(t, frozenUncachedHashNode(root), HashNode(root), "reordered children") + }) + } +} + +// This is a smoke check only: sync.Pool may discard an item or return a fresh one. +// Cleanup relies on the traversal's explicit slot/map clearing; this test does +// not identify returned items or prove that every pooled item was cleared. +func TestHashNode_PooledStorageSmoke(t *testing.T) { + root := &yaml.Node{Content: make([]*yaml.Node, 17)} + for i := range root.Content { + root.Content[i] = &yaml.Node{Content: []*yaml.Node{root, nil}, Value: strconv.Itoa(i)} + } + HashNode(root) + visited := visitedPool.Get().(map[*yaml.Node]struct{}) + assert.Empty(t, visited) + visitedPool.Put(visited) + stack := stackPool.Get().(*[]*yaml.Node) + assert.Empty(t, *stack) + for i, node := range (*stack)[:cap(*stack)] { + assert.Nil(t, node, "pooled backing slot %d", i) + } + stackPool.Put(stack) +}