perf(compilers/openapi): index the mappings a pointer descends

compilers/openapi/internal/annotation/readers_internal_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	yaml "gopkg.in/yaml.v3"
 
+	"github.com/dexpace/morphic/compilers/openapi/internal/nodeview"
 	"github.com/dexpace/morphic/ir"
 )
 
@@ -447,6 +448,63 @@ func TestRawChildNode_ReadsOnlyAMappingChild(t *testing.T) {
 	assert.Nil(t, RawChildNode(&yaml.Node{Kind: yaml.DocumentNode}, "a"), "nor an empty document")
 }
 
+// TestRawChildNode_IsNotTheMergeAwareView pins the difference between this
+// reader and nodeview's, which is the reason the two exist side by side: what a
+// keyword is preserved *as* is what the source spelled at it, while what a
+// pointer or a $ref *resolves to* is what the parser will see.
+//
+// The three cases are the three ways the trees diverge, and each is a keyword
+// this package would preserve verbatim. Answering a raw read through the view
+// would silently rewrite all of them — a merged keyword would appear at a
+// schema that never wrote it, an alias would be replaced by its target, and a
+// key written twice would change which of the two survives.
+//
+// It reaches across packages because that is where the mistake would be made:
+// nothing inside either reader can see that the other answers differently.
+func TestRawChildNode_IsNotTheMergeAwareView(t *testing.T) {
+	t.Parallel()
+
+	// use is the mapping under test in each case; the raw read of a top-level
+	// key is unambiguous, so it is safe to navigate with.
+	useOf := func(t *testing.T, src string) *yaml.Node {
+		t.Helper()
+		use := RawChildNode(yamlNode(t, src), "use")
+		require.NotNil(t, use, "the fixture must declare a `use` mapping")
+		return use
+	}
+
+	t.Run("a merge key contributes nothing to the raw read", func(t *testing.T) {
+		t.Parallel()
+		use := useOf(t, "base: &b {title: merged}\nuse:\n  <<: *b\n")
+
+		assert.Nil(t, RawChildNode(use, "title"),
+			"the source wrote `<<`, not `title`, so nothing is preserved at title")
+		merged := nodeview.New().ChildByToken(use, "title")
+		require.NotNil(t, merged, "the parser, however, does see it")
+		assert.Equal(t, "merged", merged.Value)
+	})
+
+	t.Run("an aliased value is not dereferenced by the raw read", func(t *testing.T) {
+		t.Parallel()
+		use := useOf(t, "base: &b anchored\nuse: {title: *b}\n")
+
+		raw := RawChildNode(use, "title")
+		require.NotNil(t, raw)
+		assert.Equal(t, yaml.AliasNode, raw.Kind, "the raw tree keeps the alias the source wrote")
+		assert.Equal(t, "anchored", nodeview.New().ChildByToken(use, "title").Value,
+			"where the view stands the anchor in its place")
+	})
+
+	t.Run("a repeated key resolves to opposite ends", func(t *testing.T) {
+		t.Parallel()
+		use := useOf(t, "use: {title: first, title: last}\n")
+
+		assert.Equal(t, "first", RawChildNode(use, "title").Value, "first matching key wins")
+		assert.Equal(t, "last", nodeview.New().ChildByToken(use, "title").Value,
+			"where the view follows the parser and takes the last")
+	})
+}
+
 // TestRawPropertyNode_NilSchemaReadsNothing pins the nil guard on the schema
 // side of the same reader, which every caller relies on to ask about a position
 // that may have no body written at it.

compilers/openapi/internal/nodeview/nodeview.go

@@ -47,6 +47,10 @@ const MergeDepthLimit = 64
 // expansion depth, this one caps a document with many merged mappings. Past the
 // budget the view still answers correctly — it just stops memoizing, trading a
 // cache hit for a recomputation.
+//
+// Both of the view's memos are charged to it: a mapping's expanded pairs, and
+// the key index keyIndex projects from them. One bound covering both is what
+// keeps a second memo from doubling the memory the first one was capped at.
 const maxCachedPairs = 1 << 21
 
 // DocumentRoot returns the effective root node to scan: the content of a
@@ -87,8 +91,13 @@ type Pair struct {
 // that first reached it. MergeDepthLimit and maxCachedPairs bound the chain
 // depth and cache size respectively, so unlimited memoization can't trade the
 // crash for exhausted memory instead.
+//
+// It memoizes one thing more, for the walk rather than the expansion: keyIndex
+// projects a memoized mapping into a key map, so descending a JSON pointer costs
+// a map read per token instead of a scan of every pair at each one.
 type View struct {
 	pairs       map[*yaml.Node][]Pair
+	keys        map[*yaml.Node]map[string]*yaml.Node
 	cachedPairs int
 	inFlight    map[*yaml.Node]bool
 	exhausted   bool
@@ -105,6 +114,7 @@ func (v *View) Exhausted() bool { return v.exhausted }
 func New() *View {
 	return &View{
 		pairs:    map[*yaml.Node][]Pair{},
+		keys:     map[*yaml.Node]map[string]*yaml.Node{},
 		inFlight: map[*yaml.Node]bool{},
 	}
 }
@@ -398,11 +408,7 @@ func (v *View) ChildByToken(n *yaml.Node, token string) *yaml.Node {
 	}
 	switch n.Kind {
 	case yaml.MappingNode:
-		for _, p := range v.MappingPairs(n) {
-			if p.Key == token {
-				return p.Val
-			}
-		}
+		return v.mappingChild(n, token)
 	case yaml.SequenceNode:
 		idx, err := strconv.Atoi(token)
 		if err != nil || idx < 0 || idx >= len(n.Content) {
@@ -413,6 +419,56 @@ func (v *View) ChildByToken(n *yaml.Node, token string) *yaml.Node {
 	return nil
 }
 
+// mappingChild answers one key of a mapping through the key index, falling back
+// to a scan of its pairs for a mapping the index declines to cover.
+//
+// n is known to be a mapping node here, so it is its own Deref and keys the
+// index under the same node MappingPairs memoizes the pairs under.
+func (v *View) mappingChild(n *yaml.Node, token string) *yaml.Node {
+	pairs := v.MappingPairs(n)
+	if index := v.keyIndex(n, pairs); index != nil {
+		return index[token]
+	}
+	for _, p := range pairs {
+		if p.Key == token {
+			return p.Val
+		}
+	}
+	return nil
+}
+
+// keyIndex returns n's expansion as a key map, building it on first use, or nil
+// when the view holds no memo to project.
+//
+// It is what stops a pointer walk rescanning the mappings it descends through.
+// Resolving R references into a components mapping of M entries scans R×M pairs
+// without it — quadratic in a document's own size, since both grow together —
+// where an index makes each hop a map read. A key map cannot answer differently
+// from the scan it replaces: expandContent yields each key once, so the pairs it
+// is built from hold no duplicate for a first-match scan to prefer.
+//
+// The index is charged to the pair budget and gated on it by the same test
+// memoize applies, which is what makes one bound cover both memos — and, since
+// cachedPairs only ever grows, what makes the index cover exactly the mappings
+// whose pairs the view retained: a mapping memoize declined fails this test too,
+// so there is no expansion the index keeps and the pairs do not.
+func (v *View) keyIndex(n *yaml.Node, pairs []Pair) map[string]*yaml.Node {
+	if index, built := v.keys[n]; built {
+		return index
+	}
+	if v.cachedPairs+len(pairs) > maxCachedPairs {
+		return nil
+	}
+
+	index := make(map[string]*yaml.Node, len(pairs))
+	for _, p := range pairs {
+		index[p.Key] = p.Val
+	}
+	v.keys[n] = index
+	v.cachedPairs += len(pairs)
+	return index
+}
+
 // Deref follows AliasNode links to the anchored node, bounded against an alias
 // chain that loops (the anchor-cycle detector reports those separately).
 func Deref(n *yaml.Node) *yaml.Node {

compilers/openapi/internal/nodeview/nodeview_internal_test.go

@@ -505,3 +505,97 @@ func TestPointerPath_SegmentCapStopsTheWalk(t *testing.T) {
 	assert.Len(t, path, maxPointerSegments+1,
 		"the walk stops at the cap: the root plus one node per followed token")
 }
+
+// TestChildByToken_IndexAgreesWithTheScanItReplaces holds the key index to the
+// scan it stands in for, over the mappings whose effective pairs are not their
+// literal ones: a merge source, an alias standing in for a whole mapping, and a
+// key written twice.
+//
+// A map answers by key where a scan answers by position, so the two agree only
+// because expandContent yields each key once. That is the property under test —
+// asserting the index against MappingPairs itself, key by key, is what would
+// redden if a duplicate ever survived into an expansion.
+//
+// Each mapping is read twice through one view, because the two reads take
+// different paths: the first builds the index, the second reads it back.
+func TestChildByToken_IndexAgreesWithTheScanItReplaces(t *testing.T) {
+	t.Parallel()
+	base := ymap(yscalar("a"), yscalar("1"), yscalar("b"), yscalar("2"))
+	tests := []struct {
+		name string
+		n    *yaml.Node
+	}{
+		{name: "explicit keys", n: ymap(yscalar("a"), yscalar("1"))},
+		{name: "merged keys", n: ymap(ymerge(), yalias(base), yscalar("c"), yscalar("3"))},
+		{name: "explicit beats merged", n: ymap(ymerge(), yalias(base), yscalar("a"), yscalar("9"))},
+		{name: "alias for the whole value", n: ymap(yscalar("a"), yalias(yscalar("1")))},
+		{name: "a key written twice", n: ymap(yscalar("a"), yscalar("1"), yscalar("a"), yscalar("2"))},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			v := New()
+			pairs := v.MappingPairs(tc.n)
+			require.NotEmpty(t, pairs, "the fixture must expand to something to compare")
+
+			for _, read := range []string{"builds the index", "reads it back"} {
+				for _, p := range pairs {
+					assert.Same(t, p.Val, v.ChildByToken(tc.n, p.Key), "%s: key %q", read, p.Key)
+				}
+				assert.Nil(t, v.ChildByToken(tc.n, "absent"), "%s: an unwritten key names nothing", read)
+			}
+		})
+	}
+}
+
+// TestKeyIndex_IsChargedToThePairBudget pins the index to the bound that already
+// covers the pairs it projects. A memo added outside that budget would double
+// the memory maxCachedPairs was set to cap.
+func TestKeyIndex_IsChargedToThePairBudget(t *testing.T) {
+	t.Parallel()
+	n := ymap(yscalar("a"), yscalar("1"), yscalar("b"), yscalar("2"))
+	v := New()
+
+	require.Len(t, v.MappingPairs(n), 2)
+	require.Equal(t, 2, v.cachedPairs, "the expansion is charged")
+	require.Same(t, n.Content[1], v.ChildByToken(n, "a"))
+	assert.Equal(t, 4, v.cachedPairs, "the index charges its own entries too")
+
+	require.Same(t, n.Content[3], v.ChildByToken(n, "b"))
+	assert.Equal(t, 4, v.cachedPairs, "a second read builds nothing and charges nothing")
+}
+
+// TestKeyIndex_PastTheBudgetTheScanStillAnswers covers the gate that makes the
+// index optional, from both states a read can reach it in: a mapping whose pairs
+// the budget also declined, and one memoized while the budget still allowed it.
+//
+// The second is the case the gate exists for. The first is the case that makes
+// the gate sufficient on its own: cachedPairs never falls, so a mapping memoize
+// turned away fails the identical test here, and the index cannot end up holding
+// an expansion the pairs do not.
+func TestKeyIndex_PastTheBudgetTheScanStillAnswers(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name       string
+		expandCold bool
+	}{
+		{name: "the pairs were declined too", expandCold: false},
+		{name: "the pairs were memoized first", expandCold: true},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			n := ymap(yscalar("a"), yscalar("1"))
+			v := New()
+			if tc.expandCold {
+				require.Len(t, v.MappingPairs(n), 1)
+			}
+			v.cachedPairs = maxCachedPairs
+			require.Equal(t, tc.expandCold, len(v.pairs) == 1, "the fixture must set up the state it names")
+
+			assert.Same(t, n.Content[1], v.ChildByToken(n, "a"), "the scan still answers")
+			assert.Nil(t, v.ChildByToken(n, "absent"))
+			assert.Empty(t, v.keys, "and nothing was indexed")
+		})
+	}
+}

compilers/openapi/internal/nodeview/pointerpath_bench_test.go

@@ -0,0 +1,61 @@
+package nodeview
+
+import (
+	"fmt"
+	"testing"
+
+	yaml "gopkg.in/yaml.v3"
+)
+
+// componentsDoc builds `{components: {schemas: {S0..Sn-1: {type: object}}}}`,
+// the shape every internal $ref in an OpenAPI document points into.
+func componentsDoc(n int) *yaml.Node {
+	schemas := &yaml.Node{Kind: yaml.MappingNode}
+	for i := range n {
+		schemas.Content = append(schemas.Content,
+			&yaml.Node{Kind: yaml.ScalarNode, Value: fmt.Sprintf("S%d", i)},
+			&yaml.Node{Kind: yaml.MappingNode, Content: []*yaml.Node{
+				{Kind: yaml.ScalarNode, Value: "type"},
+				{Kind: yaml.ScalarNode, Value: "object"},
+			}})
+	}
+	components := &yaml.Node{Kind: yaml.MappingNode, Content: []*yaml.Node{
+		{Kind: yaml.ScalarNode, Value: "schemas"}, schemas,
+	}}
+	return &yaml.Node{Kind: yaml.MappingNode, Content: []*yaml.Node{
+		{Kind: yaml.ScalarNode, Value: "components"}, components,
+	}}
+}
+
+// BenchmarkPointerPath_IntoAWideMapping resolves one pointer per component of a
+// components mapping, which is what the reference scan does to a document whose
+// every schema is referenced once.
+//
+// It guards a shape rather than a number. The walk descends the same mapping
+// once per reference, so the pairs it reads grow as references × components
+// without keyIndex — and those two grow together in a real document, making the
+// scan quadratic in the document's own size. Each width here does n times the
+// work of a single resolution, so the *per-component* cost is what to read:
+// divide by n and compare across widths. It should stay flat, and a run where it
+// grows with n is the index no longer being reached.
+func BenchmarkPointerPath_IntoAWideMapping(b *testing.B) {
+	for _, n := range []int{64, 256, 1024} {
+		root := componentsDoc(n)
+		pointers := make([]string, n)
+		for i := range pointers {
+			pointers[i] = fmt.Sprintf("/components/schemas/S%d", i)
+		}
+
+		b.Run(fmt.Sprintf("components%d", n), func(b *testing.B) {
+			b.ReportAllocs()
+			for b.Loop() {
+				v := New() // one view per pass: a view never outlives its compile
+				for _, p := range pointers {
+					if _, complete := v.PointerPath(root, p); !complete {
+						b.Fatalf("pointer %s must resolve", p)
+					}
+				}
+			}
+		})
+	}
+}