Repository navigation
perf: reuse a reference's quick hash before building its schema - #670
Open
josephaxisa wants to merge 1 commit into
Open
josephaxisa wants to merge 1 commit into
josephaxisa wants to merge 1 commit into
Conversation
With UseSchemaQuickHash, every non-circular reference built the schema it points to and only then found its quick hash in SchemaQuickHashMap. Many references point at the same schema, so most of those builds were thrown away: comparing test_specs/stripe.yaml with a copy carrying 300 edits, 3,954 of 5,602 reference builds during CompareDocuments found the hash already cached. Work out the key the built schema would use from the proxy and look it up first, building only on a miss. The key comes from the same function the schema uses. A node that is itself a $ref is re-attributed while building, so it still goes through Schema(). With quick hash on, CompareDocuments takes about 12% less time and 13% less memory on the stripe.yaml comparison, and about half on docusignv3.1.json. Quick hash off is unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #670 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 309 309
Lines 39279 39294 +15
=========================================
+ Hits 39279 39294 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to your #667 note that quick hash loses to no quick hash on cyclic docs.
With
UseSchemaQuickHash, each non-circular reference built its target schema before checkingSchemaQuickHashMap, so most builds were wasted: onstripe.yamlwith 300 type edits, 3,954 of 5,602 found the hash already cached. This computes the cache key from the proxy (quickHashKeyOf, split out ofquickHashKey) and builds only on a miss.$refnodes still go throughSchema().Quick hash on, median of 8 interleaved runs:
Quick hash off is unchanged (123ms), so quick hash is still slower on Stripe, but by less.
Same results as main: identical openapi-changes change sets on Stripe release pairs, full suite passes, plus a test that a cyclic spec reports the same with quick hash on and off.