diff --git a/compilers/openapi/internal/annotation/readers_internal_test.go b/compilers/openapi/internal/annotation/readers_internal_test.go index 093de97..0efe40b 100644 --- a/compilers/openapi/internal/annotation/readers_internal_test.go +++ b/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/compilers/openapi/internal/openapitest" "github.com/dexpace/morphic/ir" ) @@ -515,6 +516,74 @@ 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. +// +// Two of the cases are ways the trees diverge, and each is a keyword this +// package would preserve verbatim. Answering a raw read through the view would +// silently rewrite both — a merged keyword would appear at a schema that never +// wrote it, and an alias would be replaced by its target. +// +// The third is here because it stopped being one. A repeated key used to resolve +// to opposite ends, and GitHub #356 made the raw read take the last pair as the +// parser does, on the grounds that returning the first described a mapping by a +// value nothing else in the compiler uses. It is asserted rather than dropped so +// that the agreement is pinned: a reader drifting back to first-wins is a change +// worth failing on, not a detail to rediscover. +// +// 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(openapitest.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") + viewed := nodeview.New().ChildByToken(use, "title") + require.NotNil(t, viewed, "the view resolves the key the raw tree kept aliased") + assert.Equal(t, "anchored", viewed.Value, "where the view stands the anchor in its place") + }) + + t.Run("a repeated key resolves alike on both sides", func(t *testing.T) { + t.Parallel() + use := useOf(t, "use: {title: first, title: last}\n") + + raw, viewed := RawChildNode(use, "title"), nodeview.New().ChildByToken(use, "title") + require.NotNil(t, raw, "the raw read finds the key") + require.NotNil(t, viewed, "and so does the view") + assert.Equal(t, "last", raw.Value, + "the raw read takes the pair the parser reads, not the first written") + assert.Equal(t, "last", viewed.Value, + "and the view takes the same one, so this is no longer a divergence") + }) +} + // TestRawChildNode_FindsAKeyWrittenAsAnAlias pins the one spelling where the raw // tree and the parsed model disagree about a key's name. yaml.v3 leaves an alias // node's own Value as the anchor, so matching it raw looks for "k" while the @@ -538,9 +607,12 @@ func TestRawChildNode_FindsAKeyWrittenAsAnAlias(t *testing.T) { // last, so returning the first would describe the mapping by a value nothing // else in the compiler uses. // -// Spelled with an alias, because that is how the case is reachable — yaml.v3 -// refuses a key written twice the same way, while an explicit pair and an -// aliased one are two nodes here and one key to the parser. +// Spelled with an alias, because that is how the case is reachable from a parsed +// document — yaml.v3 refuses a key written twice when it decodes into a typed +// value, as the model parse does, so a plainly repeated key faults the document +// before any reader sees it. Decoding into a *yaml.Node, which is how a fixture +// builds a tree directly, accepts one; an explicit pair and an aliased one are +// two nodes here and one key to the parser either way. func TestRawChildNode_RepeatedKeyReadsTheLastPair(t *testing.T) { t.Parallel() for _, tc := range []struct{ name, body, want string }{ diff --git a/compilers/openapi/internal/nodeview/nodeview.go b/compilers/openapi/internal/nodeview/nodeview.go index 341aced..021178a 100644 --- a/compilers/openapi/internal/nodeview/nodeview.go +++ b/compilers/openapi/internal/nodeview/nodeview.go @@ -43,11 +43,18 @@ const maxPointerSegments = 1024 // View.expand and refCycles), not silently truncated. const MergeDepthLimit = 64 -// maxCachedPairs bounds total expanded pairs one View retains, roughly -// 50 MB at 2²¹. It complements MergeDepthLimit: that bound caps one mapping's -// 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. +// maxCachedPairs bounds total expanded pairs one View retains, on the order of +// 50 MB at 2²¹ for the pairs themselves. It complements MergeDepthLimit: that +// bound caps one mapping's 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. +// +// It bounds the key indexes beside the pairs rather than being charged twice for +// them: an index exists only for a mapping whose pairs this retained and holds +// one entry per pair, so what every index holds together is bounded by what this +// already caps. The bound is a count, and a map entry costs more than a Pair, so +// the memory ceiling with indexes in play is some multiple of the figure above +// rather than that figure. See keyIndex. const maxCachedPairs = 1 << 21 // DocumentRoot returns the effective root node to scan: the content of a @@ -88,8 +95,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 @@ -104,6 +116,8 @@ func (v *View) Exhausted() bool { return v.exhausted } // New returns an empty view; a view must not outlive the node tree whose // expansions it caches. func New() *View { + // keys is left nil: most views never index anything, and keyIndex allocates + // it on the first mapping wide enough to earn one. return &View{ pairs: map[*yaml.Node][]Pair{}, inFlight: map[*yaml.Node]bool{}, @@ -301,6 +315,13 @@ func dedupeFirstWins(pairs []Pair) []Pair { // $ref node with a type or properties sibling still drives the crash. The chain // terminates only at a node with no top-level $ref at all. func (v *View) PureRefTarget(n *yaml.Node) (string, bool) { + // Through the index where the walk already built one. This runs on every node + // a pointer descended, immediately after the walk that descended it, so a + // scan here re-reads exactly the mappings ChildByToken just stopped scanning + // — leaving the quadratic the index removes standing in its sibling. + if index := v.keys[n]; index != nil { + return pureRefFrom(index["$ref"]) + } return PureRefTargetOf(v.MappingPairs(n)) } @@ -313,14 +334,21 @@ func PureRefTargetOf(pairs []Pair) (string, bool) { if p.Key != "$ref" { continue } - if p.Val == nil || p.Val.Kind != yaml.ScalarNode { - return "", false - } - return InternalPointer(p.Val.Value) + return pureRefFrom(p.Val) } return "", false } +// pureRefFrom is the decision both readings share, once the value written at +// `$ref` is in hand: a key that is absent and one whose value is not a scalar +// are the same answer, so reading the index cannot part company with the scan. +func pureRefFrom(val *yaml.Node) (string, bool) { + if val == nil || val.Kind != yaml.ScalarNode { + return "", false + } + return InternalPointer(val.Value) +} + // InternalPointer reports the JSON pointer a $ref value names inside this // document, and whether it names this document at all. // @@ -434,17 +462,21 @@ func tokenless(pointer string) bool { // node named by one JSON pointer token, or nil when absent. The mapping arm // reads through the view, so pointer navigation resolves an alias key and an // aliased or merged value exactly as PureRefTarget does. +// +// n itself is not dereferenced, which is where this parts company with its two +// neighbours: MappingPairs and PureRefTarget both take an alias standing in for +// a whole mapping and read the mapping it names, while an alias handed here +// matches neither arm and answers nil. Every caller reaches a node through a +// walk that dereferences as it goes — PointerPath does it at each hop — so the +// difference is unreachable rather than harmless, and it is written down because +// the sibling promising the opposite is one line away. func (v *View) ChildByToken(n *yaml.Node, token string) *yaml.Node { if n == nil { return nil } 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) { @@ -455,6 +487,136 @@ 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. +// +// The built index is read before the pairs are, because on the path this exists +// to speed up they are the same answer: re-deriving the pairs first would spend +// a Deref and a memo lookup to reach a map read that never needed them. +// +// 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 { + if index := v.keys[n]; index != nil { + return index[token] + } + 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 +} + +// minIndexedPairs is the width below which a mapping is scanned rather than +// indexed. +// +// An index costs a map allocation and one insert per pair to save a comparison +// per pair per later read, so a mapping narrow enough, or read few enough times, +// never repays it — and nearly every mapping a pointer descends is both. A +// document is mostly narrow mappings: a schema body, a media-type entry, a +// response. The wide ones a walk returns to over and over are the few a +// components block holds, and those are what this admits. +// +// 16 is where the two costs meet closely enough that either side is cheap; the +// benchmark beside this file carries the widths that show it, narrow ones +// included, so a run that regresses at n=2 or n=8 is this gate having stopped +// paying for itself. +// +// Width alone is not enough, because it says nothing about reuse: a walk that +// reads a wide mapping once pays for an index it never reads again. That is not +// hypothetical — declaresResourceIDAbove builds a view per call and reads each +// node on the path exactly once, and indexing there cost it time and half again +// its allocations for nothing. So width is one of two conditions; see keyIndex +// for the other. +const minIndexedPairs = 16 + +// keyIndex returns n's expansion as a key map, building it on first use, or nil +// for a mapping this view does not index. +// +// 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. +// +// It is gated on the pairs memo rather than on a second reading of memoize's +// budget test, which is a choice about coupling rather than about behaviour: at +// the depth this runs at the two select the same mappings. Every read here enters +// expand at depth 0, where isEntryPoint holds, so a truncated expansion is +// memoized deliberately and the only expansion the budget turns away is the one +// memoize turned away for the same reason a moment earlier. A merge cycle is +// refused before memoize is reached, but it yields no pairs at all and is already +// below minIndexedPairs. +// +// The memo is still the better gate, because it is the condition itself rather +// than a restatement of it. An index is a projection of a memo entry, so "is +// there an entry" is what it has to ask; a copy of memoize's arithmetic answers +// the same today and silently stops tracking it the day memoize's own test +// changes. +// +// It is bounded by that memo rather than charged against it. An index holds one +// entry per pair of a mapping the memo kept, so the entries across every index +// are bounded by cachedPairs, which maxCachedPairs already caps. Charging them +// too would halve the memo — and that memo is not a speed budget but the bound +// that keeps a merge chain from going cubic, where the bug being fixed was a +// hang. Halving it would also bring GitHub #404 within reach at half the +// document size, since which mappings keep a memo is what decides the answer +// there. +// +// On #404 itself, which records that the pairs memo is depth-sensitive and asks +// for that to be settled before this lookup work proceeds: this index is a +// projection of that memo and holds no state of its own, so it can be neither +// more nor less correct than the entry it is built from, and it adds no second +// way for a view to answer two things. It inherits #404 rather than widening it, +// and the fix landing there fixes this with it. +// +// The second condition is reuse, and it is what the first read records rather +// than predicts. A mapping arrives here with no entry at all the first time and +// leaves with a nil one; only a read that finds that marker builds the map. So an +// index exists exactly where a walk came back, which is the only place it can be +// repaid — and a caller that touches every node once, as the resource-boundary +// walk does, allocates nothing but the markers. +// +// A nil entry cannot be mistaken for an empty index: an empty mapping has no +// pairs, and no mapping below minIndexedPairs is ever stored. +// +// The markers are bounded by the same budget the indexes are, and more tightly: +// one is written only for a mapping whose pairs the memo kept, and every such +// mapping charged at least minIndexedPairs to it, so v.keys holds at most +// maxCachedPairs/minIndexedPairs entries however many mappings a document has. +// Measured at saturation, that bound is exact. +// +// A built index is never returned from here — every caller reads v.keys itself +// before reaching this — so the marker is the only entry this has to interpret. +func (v *View) keyIndex(n *yaml.Node, pairs []Pair) map[string]*yaml.Node { + if len(pairs) < minIndexedPairs { + return nil + } + if _, memoized := v.pairs[n]; !memoized { + return nil + } + if _, seen := v.keys[n]; !seen { + if v.keys == nil { + v.keys = map[*yaml.Node]map[string]*yaml.Node{} + } + v.keys[n] = nil // read once; the next read is what earns an index + return nil + } + + index := make(map[string]*yaml.Node, len(pairs)) + for _, p := range pairs { + index[p.Key] = p.Val + } + v.keys[n] = index + 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 { diff --git a/compilers/openapi/internal/nodeview/nodeview_internal_test.go b/compilers/openapi/internal/nodeview/nodeview_internal_test.go index fddd158..40c8235 100644 --- a/compilers/openapi/internal/nodeview/nodeview_internal_test.go +++ b/compilers/openapi/internal/nodeview/nodeview_internal_test.go @@ -1,6 +1,7 @@ package nodeview import ( + "fmt" "strings" "testing" @@ -544,3 +545,213 @@ 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") } + +// wideMap builds a mapping of minIndexedPairs pairs named k0..kN-1, plus any +// extra pairs given, so a fixture reaches the width at which a view indexes. +func wideMap(extra ...*yaml.Node) *yaml.Node { + var content []*yaml.Node + for i := range minIndexedPairs { + content = append(content, + ynode.Scalar(fmt.Sprintf("k%d", i)), ynode.Scalar(fmt.Sprintf("v%d", i))) + } + return ynode.Map(append(content, extra...)...) +} + +// 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 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. +// +// Every fixture is wide enough to be indexed, and the index is asserted present +// before the reads: without that the whole table passes with the index disabled, +// comparing the fallback scan against itself. +func TestChildByToken_IndexAgreesWithTheScanItReplaces(t *testing.T) { + t.Parallel() + base := wideMap(ynode.Scalar("a"), ynode.Scalar("1"), ynode.Scalar("b"), ynode.Scalar("2")) + tests := []struct { + name string + n *yaml.Node + }{ + {name: "explicit keys", n: wideMap()}, + { + name: "merged keys", + n: wideMap(ynode.Merge(), ynode.Alias(base), ynode.Scalar("c"), ynode.Scalar("3")), + }, + { + name: "explicit beats merged", + n: wideMap(ynode.Merge(), ynode.Alias(base), ynode.Scalar("a"), ynode.Scalar("9")), + }, + {name: "an aliased value", n: wideMap(ynode.Scalar("a"), ynode.Alias(ynode.Scalar("1")))}, + { + name: "a key written twice", + n: wideMap(ynode.Scalar("a"), ynode.Scalar("1"), ynode.Scalar("a"), ynode.Scalar("2")), + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + v := New() + pairs := v.MappingPairs(tc.n) + require.GreaterOrEqual(t, len(pairs), minIndexedPairs, + "the fixture must be wide enough to index") + + 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) + require.NotNil(t, v.keys[tc.n], "%s: through the index, not the fallback scan", read) + } + }) + } +} + +// TestKeyIndex_IsBoundedByThePairsMemoRatherThanCharged pins the two halves of +// the index's bound: it holds one entry per pair of a mapping the pairs memo +// kept, and it takes nothing from that memo's own budget. +// +// Charging it would halve the memo, and that memo is not a speed budget — it is +// what keeps a merge chain from going cubic, where the bug being fixed was a +// hang. Bounding it by the memo instead costs the memo nothing and still caps +// the index, because a mapping the memo declined is never indexed at all. +func TestKeyIndex_IsBoundedByThePairsMemoRatherThanCharged(t *testing.T) { + t.Parallel() + n := wideMap() + v := New() + + pairs := v.MappingPairs(n) + require.Len(t, pairs, minIndexedPairs) + require.Equal(t, minIndexedPairs, v.cachedPairs, "the expansion is charged") + + require.Same(t, n.Content[1], v.ChildByToken(n, "k0")) + require.Nil(t, v.keys[n], "one read records the mapping without indexing it") + require.Same(t, n.Content[3], v.ChildByToken(n, "k1"), "a second read is what builds one") + + require.NotNil(t, v.keys[n], "and indexed") + assert.Equal(t, minIndexedPairs, v.cachedPairs, "the index charges the pair budget nothing") + assert.Len(t, v.keys[n], len(pairs), "one entry per pair, so the memo bounds it") +} + +// TestKeyIndex_DeclinesWhatThePairsMemoDidNotKeep covers the gate that makes the +// index optional, from each state a read can reach it in. +// +// The three states are the budget, a merge cycle, and a mapping too narrow to +// repay an index. They are asserted together because each reaches the same +// outcome down a different path, and only the first is about the budget at all: +// a cycle yields no pairs, so width turns it away before the memo is consulted. +// +// None of them distinguishes the memo gate from a copy of memoize's budget test, +// which is not what that gate is for — see keyIndex. A test claiming to pin the +// difference would be pinning nothing, since at depth 0 the two select the same +// mappings. +func TestKeyIndex_DeclinesWhatThePairsMemoDidNotKeep(t *testing.T) { + t.Parallel() + t.Run("past the budget the scan still answers", func(t *testing.T) { + t.Parallel() + n := wideMap() + v := New() + v.cachedPairs = maxCachedPairs + require.NotContains(t, v.pairs, n, "the pairs were declined") + + assert.Same(t, n.Content[1], v.ChildByToken(n, "k0"), "the scan still answers") + assert.Nil(t, v.ChildByToken(n, "absent")) + assert.Empty(t, v.keys, "and nothing was indexed, not even a marker") + }) + + t.Run("a merge cycle expands to nothing, so nothing is indexed", func(t *testing.T) { + t.Parallel() + n := wideMap() + v := New() + v.inFlight[n] = true // expand refuses a mapping already being expanded + + assert.Nil(t, v.ChildByToken(n, "k0"), "an in-flight mapping expands to nothing") + assert.NotContains(t, v.pairs, n, "and is never memoized") + assert.Empty(t, v.keys, "so there is no expansion to index") + }) + + t.Run("a mapping too narrow to repay an index is scanned", func(t *testing.T) { + t.Parallel() + n := ynode.Map(ynode.Scalar("a"), ynode.Scalar("1")) + v := New() + + assert.Same(t, n.Content[1], v.ChildByToken(n, "a"), "the scan answers") + assert.Nil(t, v.ChildByToken(n, "absent")) + assert.Empty(t, v.keys, "and nothing was indexed") + }) +} + +// TestPointerPath_ReachesTheIndexOnAWideMapping settles by assertion what the +// benchmark beside it can only show with a stopwatch. +// +// BenchmarkPointerPath_IntoAWideMapping reads a flat per-component cost as the +// index being reached, and nothing runs it in CI. What that cost depends on is +// not a timing question at all: a walk resolving many pointers through one view +// must leave an index on the mapping every one of them descends. If a gate ever +// stops admitting that mapping — a width raised, a reuse marker never promoted — +// the walk silently returns to scanning and only a benchmark nobody runs would +// show it. +// +// The pointers deliberately name distinct components, because it is the mapping +// they share that has to be indexed, not the entries they end at. +func TestPointerPath_ReachesTheIndexOnAWideMapping(t *testing.T) { + t.Parallel() + const width = minIndexedPairs + root := componentsDoc(width) + v := New() + + for i := range width { + _, complete := v.PointerPath(root, fmt.Sprintf("/components/schemas/S%d", i)) + require.True(t, complete, "every pointer resolves") + } + + schemas := v.ChildByToken(v.ChildByToken(root, "components"), "schemas") + require.NotNil(t, schemas, "the fixture has the mapping the walk descends") + require.NotNil(t, v.keys[schemas], + "the mapping every pointer descends is indexed, which a flat per-component cost rests on") + assert.Len(t, v.keys[schemas], width, "one entry per component") +} + +// TestPureRefTarget_ReadsTheIndexWhereTheWalkBuiltOne pins the sibling half of +// the walk to the same index. +// +// refScan.traverse calls this on every node a pointer descended, immediately +// after descending it, so a scan here re-reads exactly the mappings ChildByToken +// stopped scanning — which left the quadratic standing in the sibling of the +// call that removed it. Both answers are asserted through one view: the mapping +// that carries a $ref and the wide one that does not. +func TestPureRefTarget_ReadsTheIndexWhereTheWalkBuiltOne(t *testing.T) { + t.Parallel() + withRef := wideMap(ynode.Scalar("$ref"), ynode.Scalar("#/components/schemas/S")) + without := wideMap() + v := New() + + for _, n := range []*yaml.Node{withRef, without} { + require.NotNil(t, v.ChildByToken(n, "k0"), "the walk descends it") + require.Nil(t, v.keys[n], "one descent only marks it") + require.NotNil(t, v.ChildByToken(n, "k1"), "and the walk comes back") + require.NotNil(t, v.keys[n], "which is what earns the index") + } + + target, ok := v.PureRefTarget(withRef) + assert.True(t, ok) + assert.Equal(t, "/components/schemas/S", target) + + _, ok = v.PureRefTarget(without) + assert.False(t, ok, "a mapping with no $ref names no target, index or not") + + // Which side answered, rather than only what it answered. Both readings agree + // on every real document — that is the point of the index — so asserting the + // answer alone passes whether or not the index is consulted, which is how a + // test named for reading it can fail to notice it being bypassed. Planting a + // divergence is what makes the two distinguishable: only a read through the + // index can see this, and only a read through the pairs can miss it. + v.keys[withRef]["$ref"] = ynode.Scalar("#/components/schemas/Planted") + target, ok = v.PureRefTarget(withRef) + require.True(t, ok) + assert.Equal(t, "/components/schemas/Planted", target, + "the index is what answered, not a rescan of the pairs beneath it") +} diff --git a/compilers/openapi/internal/nodeview/pointerpath_bench_test.go b/compilers/openapi/internal/nodeview/pointerpath_bench_test.go new file mode 100644 index 0000000..151a000 --- /dev/null +++ b/compilers/openapi/internal/nodeview/pointerpath_bench_test.go @@ -0,0 +1,71 @@ +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. +// +// Nothing runs this in CI, so that reading is a human's. The half of it that can +// be settled without a stopwatch is settled without one: +// TestPointerPath_ReachesTheIndexOnAWideMapping asserts the walk leaves an index +// on the mapping it descends, which is the condition a flat cost depends on. +// +// The narrow widths are here because they are what a real document is mostly +// made of, and because they are the case an index loses: below minIndexedPairs +// the walk scans, and a run where these regress is that gate having stopped +// paying for itself. +func BenchmarkPointerPath_IntoAWideMapping(b *testing.B) { + for _, n := range []int{2, 8, 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) + } + } + } + }) + } +}