From 947498e5a6bd4d8424522052f8ef6c1bb556d47e Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Thu, 30 Jul 2026 20:21:26 +0300 Subject: [PATCH 1/6] openapi3: keep a document's origin tree only when it can be read (#1234) --- openapi3/loader.go | 20 ++++++++- openapi3/origin_retention_test.go | 71 +++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 openapi3/origin_retention_test.go diff --git a/openapi3/loader.go b/openapi3/loader.go index ce02e46ba..77060068f 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -149,8 +149,26 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el // rememberOriginTree retains doc's origin tree for attachOriginToResolved. // tree is nil when IncludeOrigin is off or the data took the json path. +// +// The tree is kept only for a document a $ref can reach into untyped, which is +// what attachOriginToResolved exists to re-origin. In practice that means a +// file of shared fragments, whose top level is the fragment name itself rather +// than the fields of an OpenAPI Object: +// +// User: # a $ref to "./schemas.yaml#/User" lands here, untyped +// type: object +// +// Anything OpenAPI defines a field for resolves through typed structures and +// keeps its origins on the way, so its tree could never be read. That includes +// a referenced document that is itself an OpenAPI Object: a $ref to +// "#/components/schemas/User" needs no tree. (A top-level x- extension is +// undefined by the same rule, so a document carrying one keeps its tree too, +// whether or not anything ever points at it.) +// +// Worth the condition: on a 22 MB spec the retained tree was a third of +// everything the loader held. func (loader *Loader) rememberOriginTree(doc *T, tree *originTree) { - if tree == nil { + if tree == nil || len(doc.Extensions) == 0 { return } if loader.originTrees == nil { diff --git a/openapi3/origin_retention_test.go b/openapi3/origin_retention_test.go new file mode 100644 index 000000000..0b188026f --- /dev/null +++ b/openapi3/origin_retention_test.go @@ -0,0 +1,71 @@ +package openapi3 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// An ordinary spec never consults its origin tree, so the loader does not keep +// one. Origins are still attached to the document itself; what goes away is the +// second copy the loader used to hold for the lifetime of the loader, which on +// a large spec was a third of everything it retained. +func TestOriginTree_NotRetainedForAnOrdinarySpec(t *testing.T) { + loader := NewLoader() + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/simple.yaml") + require.NoError(t, err) + + require.NotNil(t, doc.Origin, "origins are still attached to the document") + require.Empty(t, loader.originTrees, "no tree is retained: nothing could read it") +} + +// The tree is kept for exactly the documents that can consult it, and this OAD +// contains one of each: the entry document has only fields OpenAPI defines, +// while the file it $refs is a shared fragment whose top level is the schema +// name "User", which is what arrives untyped and what attachOriginToResolved +// walks the tree for. +// +// The retained tree still does its job here, so this pins the saving and the +// capability together: dropping the tree for the referenced file would leave +// the resolved schema without an origin. +func TestOriginTree_RetainedOnlyWhereItCanBeRead(t *testing.T) { + loader := NewLoader() + loader.IsExternalRefsAllowed = true + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + require.NotContains(t, loader.originTrees, doc, + "the entry document has only fields OpenAPI defines, so no $ref reaches into it untyped") + require.Len(t, loader.originTrees, 1, "only the shared-fragment file keeps its tree") + for retained := range loader.originTrees { + require.NotEmpty(t, retained.Extensions, + "a tree is kept only for a document with top-level fields OpenAPI does not define") + } + + // The capability the retained tree exists for: the resolved schema carries + // the origin of its own file, not of the $ref site. + schema := doc.Paths.Find("/users").Get.Responses.Value("200").Value. + Content["application/json"].Schema.Value + require.NotNil(t, schema.Origin, "the resolved schema keeps its origin") + require.Contains(t, schema.Origin.Key.File, "arbitrary_key_schemas.yaml") +} + +// Without IncludeOrigin there is no tree to begin with, so the new condition +// cannot change anything here. +func TestOriginTree_NotRetainedWhenOriginsAreOff(t *testing.T) { + loader := NewLoader() + loader.IsExternalRefsAllowed = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + require.Nil(t, doc.Origin) + require.Empty(t, loader.originTrees) +} From 51f520fe8bedcf067f37232bde8ad350fbf54caa Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Tue, 28 Jul 2026 02:22:34 +0300 Subject: [PATCH 2/6] openapi3: store field locations in a slice, not a map Origin.Fields was map[string]Location. A collection carries a handful of scalar fields, but a Go map allocates a whole bucket per collection whatever it holds, so the map dominated the retained size of a parsed document: on a 22 MB spec, inserting into it accounted for 294 MB of the 715 MB retained, and the maps themselves for another 22 MB. Replace it with FieldLocations, a slice in document order. Each Location already carries its Name, so the lookup key costs nothing extra, and lookup is a linear scan, which beats a map at these sizes. Retained document size drops 47% at both sizes measured: 5 MB spec: 131 MB -> 69 MB 22 MB spec: 536 MB -> 282 MB Peak heap during load is unchanged, since it is dominated by the transient yaml parse tree rather than by what is retained. Get(name) returns a location or the zero value; Lookup(name) also reports presence. MarshalJSON and UnmarshalJSON keep the serialized shape a name-keyed object, so output is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- openapi3/origin.go | 69 ++++++++++++++++++++++++++-- openapi3/origin_external_ref_test.go | 6 +-- openapi3/origin_test.go | 57 +++++++++++++---------- openapi3/validation_error.go | 2 +- openapi3/validation_error_test.go | 4 +- 5 files changed, 102 insertions(+), 36 deletions(-) diff --git a/openapi3/origin.go b/openapi3/origin.go index b571cb3d2..0879a0edf 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -1,6 +1,7 @@ package openapi3 import ( + "encoding/json" "reflect" "sort" "strings" @@ -12,14 +13,72 @@ var originPtrType = reflect.TypeFor[*Origin]() // Origin contains the origin of a collection. // Key is the location of the collection itself. -// Fields is a map of the location of each scalar field in the collection. +// Fields holds the location of each scalar field in the collection. // Sequences is a map of the location of each item in sequence-valued fields. type Origin struct { Key *Location `json:"key,omitempty" yaml:"key,omitempty"` - Fields map[string]Location `json:"fields,omitempty" yaml:"fields,omitempty"` + Fields FieldLocations `json:"fields,omitempty" yaml:"fields,omitempty"` Sequences map[string][]Location `json:"sequences,omitempty" yaml:"sequences,omitempty"` } +// FieldLocations holds the locations of a collection's scalar fields, in the +// order they appear in the document. +// +// It is a slice rather than a map[string]Location because a collection carries +// only a handful of fields, while a Go map allocates a whole bucket per +// collection whatever it holds. On a large document that overhead dominated +// the retained size of a parsed spec. Each Location already carries its Name, +// so the lookup key costs nothing extra here. +type FieldLocations []Location + +// Get returns the location of the named field, or the zero Location when the +// field has none. Use Lookup to tell an absent field from a zero location. +func (f FieldLocations) Get(name string) Location { + loc, _ := f.Lookup(name) + return loc +} + +// Lookup returns the location of the named field and whether it was found. +// The scan is linear: collections have few fields, and a linear scan over a +// contiguous slice beats a map lookup at these sizes. +func (f FieldLocations) Lookup(name string) (Location, bool) { + for i := range f { + if f[i].Name == name { + return f[i], true + } + } + return Location{}, false +} + +// MarshalJSON keeps the serialized shape a name-keyed object, as it was when +// this was a map, so the change is invisible to anything reading the output. +func (f FieldLocations) MarshalJSON() ([]byte, error) { + m := make(map[string]Location, len(f)) + for _, loc := range f { + m[loc.Name] = loc + } + return json.Marshal(m) +} + +// UnmarshalJSON reads the name-keyed object written by MarshalJSON. Entries are +// sorted by name, since a JSON object carries no order to restore. +func (f *FieldLocations) UnmarshalJSON(data []byte) error { + var m map[string]Location + if err := json.Unmarshal(data, &m); err != nil { + return err + } + out := make(FieldLocations, 0, len(m)) + for name, loc := range m { + if loc.Name == "" { + loc.Name = name + } + out = append(out, loc) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + *f = out + return nil +} + // Location is a struct that contains the location of a field. type Location struct { File string `json:"file,omitempty" yaml:"file,omitempty"` @@ -61,17 +120,17 @@ func originFromSeq(s []any) *Origin { nf := toInt(s[idx]) idx++ if nf > 0 && idx+nf*3 <= len(s) { - o.Fields = make(map[string]Location, nf) + o.Fields = make(FieldLocations, 0, nf) for range nf { fname, _ := s[idx].(string) delta := toInt(s[idx+1]) col := toInt(s[idx+2]) - o.Fields[fname] = Location{ + o.Fields = append(o.Fields, Location{ File: file, Line: keyLine + delta, Column: col, Name: fname, - } + }) idx += 3 } } diff --git a/openapi3/origin_external_ref_test.go b/openapi3/origin_external_ref_test.go index 83f7ea019..40a562706 100644 --- a/openapi3/origin_external_ref_test.go +++ b/openapi3/origin_external_ref_test.go @@ -35,14 +35,14 @@ func TestOrigin_ExternalRefToArbitraryTopLevelKey(t *testing.T) { Name: "User", EndLine: 7, EndColumn: 19, }, *user.Origin.Key, "the key origin spans the whole User block in arbitrary_key_schemas.yaml") - require.Equal(t, 2, user.Origin.Fields["type"].Line, "field origins are attached too") + require.Equal(t, 2, user.Origin.Fields.Get("type").Line, "field origins are attached too") // the subtree gets origins as well, with the same file id := user.Properties["id"].Value require.NotNil(t, id.Origin) require.Equal(t, user.Origin.Key.File, id.Origin.Key.File) require.Equal(t, 4, id.Origin.Key.Line, "the id property's own line") - require.Equal(t, 5, id.Origin.Fields["type"].Line) + require.Equal(t, 5, id.Origin.Fields.Get("type").Line) } // Re-attaching origins reuses the origin tree retained at load time: resolving @@ -101,6 +101,6 @@ User: require.Equal(t, "User", user.Origin.Key.Name) require.Equal(t, 14, user.Origin.Key.Line, "the User: line in the document above") require.Equal(t, 17, user.Origin.Key.EndLine, "the block's last line") - require.Equal(t, 15, user.Origin.Fields["type"].Line) + require.Equal(t, 15, user.Origin.Fields.Get("type").Line) require.Empty(t, user.Origin.Key.File, "a document loaded from data has no file") } diff --git a/openapi3/origin_test.go b/openapi3/origin_test.go index 2781a655b..8db83c3f5 100644 --- a/openapi3/origin_test.go +++ b/openapi3/origin_test.go @@ -28,7 +28,7 @@ func TestOrigin_T(t *testing.T) { Column: 1, Name: "openapi", }, - doc.Origin.Fields["openapi"]) + doc.Origin.Fields.Get("openapi")) } func TestOrigin_Info(t *testing.T) { @@ -59,7 +59,7 @@ func TestOrigin_Info(t *testing.T) { Column: 3, Name: "title", }, - doc.Info.Origin.Fields["title"]) + doc.Info.Origin.Fields.Get("title")) require.Equal(t, openapi3.Location{ @@ -68,7 +68,7 @@ func TestOrigin_Info(t *testing.T) { Column: 3, Name: "version", }, - doc.Info.Origin.Fields["version"]) + doc.Info.Origin.Fields.Get("version")) } func TestOrigin_Paths(t *testing.T) { @@ -211,7 +211,7 @@ func TestOrigin_Responses(t *testing.T) { Column: 11, Name: "description", }, - base.Value("200").Value.Origin.Fields["description"]) + base.Value("200").Value.Origin.Fields.Get("description")) } func TestOrigin_Parameters(t *testing.T) { @@ -243,7 +243,7 @@ func TestOrigin_Parameters(t *testing.T) { Column: 11, Name: "in", }, - base.Origin.Fields["in"]) + base.Origin.Fields.Get("in")) require.Equal(t, openapi3.Location{ @@ -252,7 +252,7 @@ func TestOrigin_Parameters(t *testing.T) { Column: 11, Name: "name", }, - base.Origin.Fields["name"]) + base.Origin.Fields.Get("name")) } func TestOrigin_SchemaInAdditionalProperties(t *testing.T) { @@ -286,7 +286,7 @@ func TestOrigin_SchemaInAdditionalProperties(t *testing.T) { Column: 19, Name: "type", }, - base.Schema.Value.Origin.Fields["type"]) + base.Schema.Value.Origin.Fields.Get("type")) } func TestOrigin_ExternalDocs(t *testing.T) { @@ -319,7 +319,7 @@ func TestOrigin_ExternalDocs(t *testing.T) { Column: 3, Name: "description", }, - base.Origin.Fields["description"]) + base.Origin.Fields.Get("description")) require.Equal(t, openapi3.Location{ @@ -328,7 +328,7 @@ func TestOrigin_ExternalDocs(t *testing.T) { Column: 3, Name: "url", }, - base.Origin.Fields["url"]) + base.Origin.Fields.Get("url")) } func TestOrigin_Security(t *testing.T) { @@ -361,7 +361,7 @@ func TestOrigin_Security(t *testing.T) { Column: 7, Name: "type", }, - base.Origin.Fields["type"]) + base.Origin.Fields.Get("type")) require.Equal(t, &openapi3.Location{ @@ -392,7 +392,7 @@ func TestOrigin_Security(t *testing.T) { Column: 11, Name: "authorizationUrl", }, - base.Flows.Implicit.Origin.Fields["authorizationUrl"]) + base.Flows.Implicit.Origin.Fields.Get("authorizationUrl")) // scopes is a map[string]string, which decodes without an Origin of its own, // so its per-key locations are recorded on the flow's Origin as a named @@ -434,7 +434,7 @@ func TestOrigin_Example(t *testing.T) { Column: 17, Name: "summary", }, - base.Origin.Fields["summary"]) + base.Origin.Fields.Get("summary")) // Example.Value is an any-typed field, so __origin__ is stripped from it during unmarshaling. require.NotContains(t, @@ -471,7 +471,7 @@ func TestOrigin_XML(t *testing.T) { Column: 21, Name: "namespace", }, - base.Origin.Fields["namespace"]) + base.Origin.Fields.Get("namespace")) require.Equal(t, openapi3.Location{ @@ -480,7 +480,7 @@ func TestOrigin_XML(t *testing.T) { Column: 21, Name: "prefix", }, - base.Origin.Fields["prefix"]) + base.Origin.Fields.Get("prefix")) } // TestOrigin_AnyFieldsStripped verifies that __origin__ is absent from all @@ -672,7 +672,7 @@ func TestOrigin_WithExternalRef(t *testing.T) { Column: 3, Name: "namespace", }, - base.XML.Origin.Fields["namespace"]) + base.XML.Origin.Fields.Get("namespace")) require.Equal(t, openapi3.Location{ @@ -681,7 +681,7 @@ func TestOrigin_WithExternalRef(t *testing.T) { Column: 3, Name: "prefix", }, - base.XML.Origin.Fields["prefix"]) + base.XML.Origin.Fields.Get("prefix")) } // TestOrigin_WithExternalRefRootOrigin verifies that the root-level schema of an @@ -721,7 +721,7 @@ func TestOrigin_WithExternalRefRootOrigin(t *testing.T) { Column: 1, Name: "type", }, - base.Origin.Fields["type"]) + base.Origin.Fields.Get("type")) } // TestOrigin_MaplikeNoOriginKey verifies that __origin__ does not appear as a @@ -776,7 +776,7 @@ func TestOrigin_RequiredSequence(t *testing.T) { require.NotNil(t, schema.Origin) // "required" must appear in Fields (it's a sequence-valued field) - require.Contains(t, schema.Origin.Fields, "required") + require.True(t, mustHaveField(schema.Origin.Fields, "required")) // Sequences must record per-item locations for "required" seqLocs, ok := schema.Origin.Sequences["required"] @@ -854,7 +854,7 @@ func TestOrigin_Headers(t *testing.T) { Column: 15, Name: "description", }, - headers["X-Rate-Limit"].Value.Origin.Fields["description"]) + headers["X-Rate-Limit"].Value.Origin.Fields.Get("description")) require.Equal(t, &openapi3.Location{ @@ -941,29 +941,36 @@ func TestOrigin_MappingFields(t *testing.T) { file := "testdata/origin/mapping_fields.yaml" // dependentRequired is a map[string][]string — mapping-valued - require.Contains(t, schema.Origin.Fields, "dependentRequired") + require.True(t, mustHaveField(schema.Origin.Fields, "dependentRequired")) require.Equal(t, openapi3.Location{ File: file, Line: 18, Column: 21, Name: "dependentRequired", - }, schema.Origin.Fields["dependentRequired"]) + }, schema.Origin.Fields.Get("dependentRequired")) // dependentSchemas is a Schemas map — mapping-valued - require.Contains(t, schema.Origin.Fields, "dependentSchemas") + require.True(t, mustHaveField(schema.Origin.Fields, "dependentSchemas")) require.Equal(t, openapi3.Location{ File: file, Line: 22, Column: 21, Name: "dependentSchemas", - }, schema.Origin.Fields["dependentSchemas"]) + }, schema.Origin.Fields.Get("dependentSchemas")) // patternProperties is a Schemas map — mapping-valued - require.Contains(t, schema.Origin.Fields, "patternProperties") + require.True(t, mustHaveField(schema.Origin.Fields, "patternProperties")) require.Equal(t, openapi3.Location{ File: file, Line: 25, Column: 21, Name: "patternProperties", - }, schema.Origin.Fields["patternProperties"]) + }, schema.Origin.Fields.Get("patternProperties")) +} + +// mustHaveField reports whether the named field carries a location, the +// slice-shaped equivalent of asserting a key is present in a map. +func mustHaveField(f openapi3.FieldLocations, name string) bool { + _, ok := f.Lookup(name) + return ok } diff --git a/openapi3/validation_error.go b/openapi3/validation_error.go index f2954afb3..f5e78e2fa 100644 --- a/openapi3/validation_error.go +++ b/openapi3/validation_error.go @@ -1424,7 +1424,7 @@ func exampleValueOrigin(ex *Example, fallback *Origin) *Origin { if ex == nil || ex.Origin == nil { return fallback } - if loc, ok := ex.Origin.Fields["value"]; ok { + if loc, ok := ex.Origin.Fields.Lookup("value"); ok { return &Origin{Key: &loc} } return ex.Origin diff --git a/openapi3/validation_error_test.go b/openapi3/validation_error_test.go index ebae2a290..21bfbfa18 100644 --- a/openapi3/validation_error_test.go +++ b/openapi3/validation_error_test.go @@ -382,8 +382,8 @@ paths: {} require.Equal(t, "openapi", rfe.Field) require.NotNil(t, rfe.Origin, "doc-root fields now carry the document's Origin") require.Same(t, doc.Origin, rfe.Origin, "the error carries T.Origin") - require.Greater(t, rfe.Origin.Fields["openapi"].Line, 0, - `Origin.Fields["openapi"] locates the openapi: line`) + require.Greater(t, rfe.Origin.Fields.Get("openapi").Line, 0, + `Origin.Fields.Get("openapi") locates the openapi: line`) } // SchemaValueError clusters "'s example/default value From cde4b0927bf14ebec8d5265297cf61289d696187 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sat, 1 Aug 2026 17:11:32 +0300 Subject: [PATCH 3/6] openapi3: hand over the field slice instead of copying it recordMapKeyLocations built a whole *Origin -- key Location, fields slice, sequences map -- purely to read its Fields, then copied that slice into a second one and discarded the rest. Nothing else holds the slice, so it can be sorted in place and handed over directly, removing one allocation per scalar-valued map. --- openapi3/origin.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/openapi3/origin.go b/openapi3/origin.go index 0879a0edf..18dd7a987 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -207,10 +207,13 @@ func isScalarValuedMapField(v reflect.Value) bool { return false } -// recordMapKeyLocations copies the map-key locations from a scalar-valued map's +// recordMapKeyLocations moves the map-key locations from a scalar-valued map's // own subtree onto parentOrigin.Sequences[field], so each key is addressable by // name (the same shape used for sequence items). It is a no-op when the child // carries no origin data. Keys are sorted for deterministic output. +// +// childOrigin is discarded here, and nothing else holds its Fields, so the +// slice is sorted and handed over in place rather than copied. func recordMapKeyLocations(parentOrigin *Origin, field string, childTree *yaml.OriginTree) { s, ok := childTree.Origin.([]any) if !ok { @@ -220,10 +223,7 @@ func recordMapKeyLocations(parentOrigin *Origin, field string, childTree *yaml.O if childOrigin == nil || len(childOrigin.Fields) == 0 { return } - locs := make([]Location, 0, len(childOrigin.Fields)) - for _, loc := range childOrigin.Fields { - locs = append(locs, loc) - } + locs := childOrigin.Fields sort.Slice(locs, func(i, j int) bool { return locs[i].Name < locs[j].Name }) if parentOrigin.Sequences == nil { parentOrigin.Sequences = make(map[string][]Location) From 9a8c4c0249d8939e98fa33e5243ec8f2e8a3bffb Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 2 Aug 2026 00:45:13 +0300 Subject: [PATCH 4/6] docs: regenerate the API docs for FieldLocations ./docs.sh output is checked in and verified by CI's git diff --exit-code; changing Origin.Fields's type and adding Get/Lookup changed the public API surface. --- .github/docs/openapi3.txt | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index f710e8524..f08e0df01 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -991,6 +991,33 @@ func (e *ExtraSiblingFieldsError) Code() string func (e *ExtraSiblingFieldsError) Error() string +type FieldLocations []Location + FieldLocations holds the locations of a collection's scalar fields, in the + order they appear in the document. + + It is a slice rather than a map[string]Location because a collection carries + only a handful of fields, while a Go map allocates a whole bucket per + collection whatever it holds. On a large document that overhead dominated + the retained size of a parsed spec. Each Location already carries its Name, + so the lookup key costs nothing extra here. + +func (f FieldLocations) Get(name string) Location + Get returns the location of the named field, or the zero Location when the + field has none. Use Lookup to tell an absent field from a zero location. + +func (f FieldLocations) Lookup(name string) (Location, bool) + Lookup returns the location of the named field and whether it was found. + The scan is linear: collections have few fields, and a linear scan over a + contiguous slice beats a map lookup at these sizes. + +func (f FieldLocations) MarshalJSON() ([]byte, error) + MarshalJSON keeps the serialized shape a name-keyed object, as it was when + this was a map, so the change is invisible to anything reading the output. + +func (f *FieldLocations) UnmarshalJSON(data []byte) error + UnmarshalJSON reads the name-keyed object written by MarshalJSON. Entries + are sorted by name, since a JSON object carries no order to restore. + type FieldVersionMismatchError struct { // Field is the field name flagged (e.g. "summary", "identifier", // "$defs", "prefixItems", "contains", ...). @@ -1872,11 +1899,11 @@ func (e *OperationValidationError) Unwrap() error type Origin struct { Key *Location `json:"key,omitempty" yaml:"key,omitempty"` - Fields map[string]Location `json:"fields,omitempty" yaml:"fields,omitempty"` + Fields FieldLocations `json:"fields,omitempty" yaml:"fields,omitempty"` Sequences map[string][]Location `json:"sequences,omitempty" yaml:"sequences,omitempty"` } - Origin contains the origin of a collection. Key is the location of the - collection itself. Fields is a map of the location of each scalar field + Origin contains the origin of a collection. Key is the location of + the collection itself. Fields holds the location of each scalar field in the collection. Sequences is a map of the location of each item in sequence-valued fields. From db614a920e92cb57581da0d28e5021855ea6ed75 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 2 Aug 2026 13:21:12 +0300 Subject: [PATCH 5/6] openapi3: use slices.SortFunc, and say why Lookup is a hand loop sort.Slice swaps through reflection; slices.SortFunc is generic and the package is already used elsewhere in the repo. Both call sites are name-ordering, one on the load path (recordMapKeyLocations) and one on the JSON round-trip. Lookup deliberately stays a hand-written loop. slices.IndexFunc reads better but its closure does not inline, so it pays a call per element: measured on 3/6/12 fields it is 5-100% slower on a hit and 2-3x slower on a miss, and misses dominate here since most fields carry no recorded location. The comment records that so it is not 'modernised' later. --- .github/docs/openapi3.txt | 5 +++++ openapi3/origin.go | 11 ++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index f08e0df01..3a43cb523 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -1010,6 +1010,11 @@ func (f FieldLocations) Lookup(name string) (Location, bool) The scan is linear: collections have few fields, and a linear scan over a contiguous slice beats a map lookup at these sizes. + Deliberately a hand-written loop rather than slices.IndexFunc: the closure + does not inline, so IndexFunc pays a call per element. Measured on 3/6/12 + fields it is 5-100% slower on a hit and 2-3x slower on a miss, and misses + are the common case here (most fields carry no recorded location). + func (f FieldLocations) MarshalJSON() ([]byte, error) MarshalJSON keeps the serialized shape a name-keyed object, as it was when this was a map, so the change is invisible to anything reading the output. diff --git a/openapi3/origin.go b/openapi3/origin.go index 18dd7a987..e79d50df9 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -3,7 +3,7 @@ package openapi3 import ( "encoding/json" "reflect" - "sort" + "slices" "strings" "github.com/oasdiff/yaml" @@ -41,6 +41,11 @@ func (f FieldLocations) Get(name string) Location { // Lookup returns the location of the named field and whether it was found. // The scan is linear: collections have few fields, and a linear scan over a // contiguous slice beats a map lookup at these sizes. +// +// Deliberately a hand-written loop rather than slices.IndexFunc: the closure +// does not inline, so IndexFunc pays a call per element. Measured on 3/6/12 +// fields it is 5-100% slower on a hit and 2-3x slower on a miss, and misses +// are the common case here (most fields carry no recorded location). func (f FieldLocations) Lookup(name string) (Location, bool) { for i := range f { if f[i].Name == name { @@ -74,7 +79,7 @@ func (f *FieldLocations) UnmarshalJSON(data []byte) error { } out = append(out, loc) } - sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + slices.SortFunc(out, func(a, b Location) int { return strings.Compare(a.Name, b.Name) }) *f = out return nil } @@ -224,7 +229,7 @@ func recordMapKeyLocations(parentOrigin *Origin, field string, childTree *yaml.O return } locs := childOrigin.Fields - sort.Slice(locs, func(i, j int) bool { return locs[i].Name < locs[j].Name }) + slices.SortFunc(locs, func(a, b Location) int { return strings.Compare(a.Name, b.Name) }) if parentOrigin.Sequences == nil { parentOrigin.Sequences = make(map[string][]Location) } From 1fbda1771bd55916066eee93e5d524a304f3357d Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 2 Aug 2026 13:57:13 +0300 Subject: [PATCH 6/6] openapi3: record why Sequences stays a map The obvious question on this change is why Fields moved off a map and Sequences did not. Two reasons, now written where a future reader will find them rather than only in the pull request. FieldLocations can drop the map because Location.Name already carries the key, so the map stored what the value repeated. In Sequences, Location.Name holds the item's value (an enum member, a required property) while the key is the field's name, so a slice would need a wrapper type invented purely to hold it -- trading a plain slice of an existing type for a slice of structs each holding a slice. The memory argument is also far weaker: only a collection with a sequence-valued field allocates one, measured at 5% of collections (20,000 of 390,004) on a 16.8 MB spec, and a nil map costs nothing. --- .github/docs/openapi3.txt | 9 +++++++++ openapi3/origin.go | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 3a43cb523..81be295f4 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -1912,6 +1912,15 @@ type Origin struct { in the collection. Sequences is a map of the location of each item in sequence-valued fields. + Sequences stays a map although Fields is a slice, which is deliberate. + FieldLocations drops the map because Location.Name already carries the key, + so the map was storing information the value repeated. Here Location.Name + holds the *item's* value (an enum member, a required property) while the key + is the *field's* name ("enum", "required", "tags"), so a slice would need a + wrapper type invented to hold it. The memory argument is also much weaker: + only a collection with a sequence-valued field allocates one at all, + which measured at 5% of collections on a large spec, and a nil map is free. + type Parameter struct { Extensions map[string]any `json:"-" yaml:"-"` Origin *Origin `json:"-" yaml:"-"` diff --git a/openapi3/origin.go b/openapi3/origin.go index e79d50df9..1379a7c76 100644 --- a/openapi3/origin.go +++ b/openapi3/origin.go @@ -15,6 +15,15 @@ var originPtrType = reflect.TypeFor[*Origin]() // Key is the location of the collection itself. // Fields holds the location of each scalar field in the collection. // Sequences is a map of the location of each item in sequence-valued fields. +// +// Sequences stays a map although Fields is a slice, which is deliberate. +// FieldLocations drops the map because Location.Name already carries the key, +// so the map was storing information the value repeated. Here Location.Name +// holds the *item's* value (an enum member, a required property) while the key +// is the *field's* name ("enum", "required", "tags"), so a slice would need a +// wrapper type invented to hold it. The memory argument is also much weaker: +// only a collection with a sequence-valued field allocates one at all, which +// measured at 5% of collections on a large spec, and a nil map is free. type Origin struct { Key *Location `json:"key,omitempty" yaml:"key,omitempty"` Fields FieldLocations `json:"fields,omitempty" yaml:"fields,omitempty"`