perf: reuse a reference's quick hash before building its schema

datamodel/low/base/schema_hash.go

@@ -10,7 +10,9 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/pb33f/go-yaml"
 	"github.com/pb33f/libopenapi/datamodel/low"
+	"github.com/pb33f/libopenapi/index"
 	"github.com/pb33f/libopenapi/orderedmap"
 )
 
@@ -412,36 +414,39 @@ func hashSchemaDependentRequired(h *maphash.Hash, label string, m *orderedmap.Ma
 }
 
 func (s *Schema) quickHashKey() string {
-	// The key identifies a node by file plus position, so the path must come from the file
-	// RootNode is in. GetIndex() may have been re-attributed to the file a $ref resolves to,
-	// which would pair a resolved-file path with a referring-file line and column: two
-	// different $refs at the same position in different files pointing into one shared file
-	// would then collide and return each other's hash.
-	// refIndex is assigned unconditionally in Build alongside Index, so it is nil only when
-	// the schema was never built, in which case Index is nil too and there is no path either way.
-	idx := s.refIndex
+	return quickHashKeyOf(s.refIndex, s.Index, s.RootNode)
+}
+
+// quickHashKeyOf builds the quick-hash key for a schema whose root node is root, read from the file of
+// refIndex, in the document of idx. The key identifies a node by file plus position, so the path must come
+// from the file root is in. Index may have been re-attributed to the file a $ref resolves to, which would
+// pair a resolved-file path with a referring-file line and column: two different $refs at the same position
+// in different files pointing into one shared file would then collide and return each other's hash.
+// refIndex is assigned unconditionally in Build alongside Index, so it is nil only when the schema was never
+// built, in which case Index is nil too and there is no path either way.
+func quickHashKeyOf(refIndex, idx *index.SpecIndex, root *yaml.Node) string {
 	path := ""
-	if idx != nil {
-		path = idx.GetSpecAbsolutePath()
+	if refIndex != nil {
+		path = refIndex.GetSpecAbsolutePath()
 	}
 	cfID := "root"
-	if s.Index != nil {
-		if s.Index.GetRolodex() != nil {
-			if s.Index.GetRolodex().GetId() != "" {
-				cfID = s.Index.GetRolodex().GetId()
+	if idx != nil {
+		if idx.GetRolodex() != nil {
+			if idx.GetRolodex().GetId() != "" {
+				cfID = idx.GetRolodex().GetId()
 			}
 		} else {
-			cfID = s.Index.GetConfig().GetId()
+			cfID = idx.GetConfig().GetId()
 		}
 	}
 
 	var keyBuf strings.Builder
 	keyBuf.Grow(len(path) + len(cfID) + 16)
 	keyBuf.WriteString(path)
 	keyBuf.WriteByte(':')
-	keyBuf.WriteString(strconv.Itoa(s.RootNode.Line))
+	keyBuf.WriteString(strconv.Itoa(root.Line))
 	keyBuf.WriteByte(':')
-	keyBuf.WriteString(strconv.Itoa(s.RootNode.Column))
+	keyBuf.WriteString(strconv.Itoa(root.Column))
 	keyBuf.WriteByte(':')
 	keyBuf.WriteString(cfID)
 	return keyBuf.String()

datamodel/low/base/schema_proxy.go

@@ -267,7 +267,8 @@ func (sp *SchemaProxy) Schema() *Schema {
 // there were no errors during build, then nil will be returned.
 //
 // Thread safety: GetBuildError() is safe to call concurrently only after Schema() has been called at least once
-// on this proxy (from any goroutine). All standard code paths (Hash(), high-level Schema()) call Schema() first.
+// on this proxy (from any goroutine). The high-level Schema() calls Schema() first; Hash() may not, when a quick
+// hash for the referenced schema is already cached.
 func (sp *SchemaProxy) GetBuildError() error {
 	return sp.buildError
 }
@@ -363,6 +364,10 @@ func (sp *SchemaProxy) computeHash() uint64 {
 	cfg := sp.getSpecConfig()
 	if cfg != nil && cfg.UseSchemaQuickHash {
 		if !CheckSchemaProxyForCircularRefs(sp) {
+			// many references point at the same schema; reuse its quick hash before building another copy.
+			if hash, ok := sp.cachedQuickHash(); ok {
+				return hash
+			}
 			sch = sp.Schema()
 			if sch != nil {
 				return sch.QuickHash()
@@ -475,3 +480,26 @@ func (sp *SchemaProxy) attemptPropertyMerging(node *yaml.Node, config *datamodel
 
 	return merged
 }
+
+// cachedQuickHash returns the quick hash already stored for the schema this reference would build, without
+// building it. It only answers when the key the built schema would use can be worked out from the proxy: a node
+// that is itself a $ref is re-attributed to another index while building (and is the only kind of node
+// referenced-property merging changes), so that is left to Schema().
+func (sp *SchemaProxy) cachedQuickHash() (uint64, bool) {
+	if sp.vn == nil {
+		return 0, false
+	}
+	root := utils.ResolveMergeNodes(sp.vn)
+	if root == nil {
+		return 0, false
+	}
+	if isRef, _, _ := utils.IsNodeRefValue(root); isRef {
+		return 0, false
+	}
+	if v, ok := SchemaQuickHashMap.Load(quickHashKeyOf(sp.idx, sp.idx, root)); ok {
+		if hash, isHash := v.(uint64); isHash {
+			return hash, true
+		}
+	}
+	return 0, false
+}

datamodel/low/base/schema_proxy_test.go

@@ -921,3 +921,80 @@ properties:
 	recomputedHash := sch.Hash()
 	assert.NotEqual(t, uint64(0), recomputedHash)
 }
+
+// quickHashRefProxy is a reference proxy as the model builds it: its value node is the schema it points to.
+func quickHashRefProxy(t *testing.T, target string, cfg *index.SpecIndexConfig) *SchemaProxy {
+	t.Helper()
+	var node yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(target), &node))
+	cfg.UseSchemaQuickHash = true
+	idx := index.NewSpecIndexWithConfig(&node, cfg)
+	rolodex := index.NewRolodex(cfg)
+	idx.SetRolodex(rolodex)
+	rolodex.SetRootIndex(idx)
+
+	sp := new(SchemaProxy)
+	sp.vn = node.Content[0]
+	sp.idx = idx
+	sp.ctx = context.Background()
+	r := low.Reference{}
+	r.SetReference("#/components/schemas/Pet", &yaml.Node{})
+	sp.Reference = r
+	return sp
+}
+
+func TestSchemaProxy_QuickHash_ReusesCachedHashWithoutBuilding(t *testing.T) {
+	ClearSchemaQuickHashMap()
+	defer ClearSchemaQuickHashMap()
+	sp := quickHashRefProxy(t, "type: object\nproperties:\n  name:\n    type: string\n", &index.SpecIndexConfig{})
+
+	SchemaQuickHashMap.Store(quickHashKeyOf(sp.idx, sp.idx, sp.vn), uint64(42))
+
+	assert.Equal(t, uint64(42), sp.Hash())
+	assert.Nil(t, sp.rendered.Load(), "the referenced schema should not be built")
+}
+
+func TestSchemaProxy_QuickHash_BuildsOnMiss(t *testing.T) {
+	ClearSchemaQuickHashMap()
+	defer ClearSchemaQuickHashMap()
+	sp := quickHashRefProxy(t, "type: object\nproperties:\n  name:\n    type: string\n", &index.SpecIndexConfig{})
+
+	hash := sp.Hash()
+
+	built := sp.rendered.Load()
+	require.NotNil(t, built)
+	assert.Equal(t, built.QuickHash(), hash)
+	// the key worked out from the proxy is the one the built schema uses.
+	assert.Equal(t, built.quickHashKey(), quickHashKeyOf(sp.idx, sp.idx, sp.vn))
+}
+
+func TestSchemaProxy_CachedQuickHash_Declines(t *testing.T) {
+	ClearSchemaQuickHashMap()
+	defer ClearSchemaQuickHashMap()
+	target := "type: object\nproperties:\n  name:\n    type: string\n"
+
+	// a node that is itself a $ref is re-attributed while building, with or without property merging.
+	for _, cfg := range []*index.SpecIndexConfig{{}, {MergeReferencedProperties: true}} {
+		sp := quickHashRefProxy(t, "$ref: '#/components/schemas/Other'\ndescription: sibling\n", cfg)
+		SchemaQuickHashMap.Store(quickHashKeyOf(sp.idx, sp.idx, sp.vn), uint64(42))
+		_, ok := sp.cachedQuickHash()
+		assert.False(t, ok)
+	}
+
+	// nothing to build.
+	sp := new(SchemaProxy)
+	_, ok := sp.cachedQuickHash()
+	assert.False(t, ok)
+
+	// an alias with nothing behind it.
+	sp = quickHashRefProxy(t, target, &index.SpecIndexConfig{})
+	sp.vn = &yaml.Node{Kind: yaml.AliasNode}
+	_, ok = sp.cachedQuickHash()
+	assert.False(t, ok)
+
+	// a value that isn't a hash.
+	sp = quickHashRefProxy(t, target, &index.SpecIndexConfig{})
+	SchemaQuickHashMap.Store(quickHashKeyOf(sp.idx, sp.idx, sp.vn), "not a hash")
+	_, ok = sp.cachedQuickHash()
+	assert.False(t, ok)
+}

what-changed/what_changed_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/pb33f/libopenapi/datamodel"
 	v2 "github.com/pb33f/libopenapi/datamodel/low/v2"
 	v3 "github.com/pb33f/libopenapi/datamodel/low/v3"
+	"github.com/pb33f/libopenapi/what-changed/model"
 	"github.com/pb33f/testify/assert"
 	"github.com/pb33f/testify/require"
 )
@@ -388,3 +389,68 @@ func TestCompareOpenAPIDocuments_CrossFileCircularReference(t *testing.T) {
 	assert.Equal(t, 1, changes.TotalChanges())
 	assert.Equal(t, 1, changes.TotalBreakingChanges())
 }
+
+// Many references to one schema in a document with a reference loop: with quick hash, references after the
+// first reuse the cached hash of the schema they point to rather than building it again. The result must be the
+// same as without quick hash.
+func TestCompareOpenAPIDocuments_QuickHashCyclicSharedRefs(t *testing.T) {
+	spec := func(nameType string) []byte {
+		return []byte(`openapi: 3.1.0
+info:
+  title: shared refs
+  version: '1'
+paths: {}
+components:
+  schemas:
+    Pet:
+      type: object
+      properties:
+        name:
+          type: ` + nameType + `
+    Owner:
+      type: object
+      properties:
+        pet:
+          $ref: '#/components/schemas/Pet'
+        pets:
+          type: array
+          items:
+            $ref: '#/components/schemas/Pet'
+    Shop:
+      type: object
+      properties:
+        featured:
+          $ref: '#/components/schemas/Pet'
+        owner:
+          $ref: '#/components/schemas/Owner'
+    Node:
+      type: object
+      properties:
+        next:
+          $ref: '#/components/schemas/Link'
+    Link:
+      type: object
+      properties:
+        node:
+          $ref: '#/components/schemas/Node'`)
+	}
+	compare := func(quick bool) *model.DocumentChanges {
+		config := datamodel.NewDocumentConfiguration()
+		config.UseSchemaQuickHash = quick
+		load := func(b []byte) *v3.Document {
+			info, err := datamodel.ExtractSpecInfo(b)
+			require.NoError(t, err)
+			doc, err := v3.CreateDocumentFromConfig(info, config)
+			require.NoError(t, err)
+			return doc
+		}
+		return CompareOpenAPIDocuments(load(spec("string")), load(spec("integer")))
+	}
+
+	withQuickHash, without := compare(true), compare(false)
+	require.NotNil(t, withQuickHash)
+	require.NotNil(t, without)
+	assert.Equal(t, without.TotalChanges(), withQuickHash.TotalChanges())
+	assert.Equal(t, without.TotalBreakingChanges(), withQuickHash.TotalBreakingChanges())
+	assert.Equal(t, 1, withQuickHash.TotalBreakingChanges()) // Pet.name: string -> integer
+}