Repository navigation
Conversation
`isKeywordEnabled` runs per keyword per node during validation. Each call scanned every entry of `vocabularyKeywords` and ran `includes` over its keyword array. Build the reverse index once at module load and look the keyword up directly. The core-keyword check moves above the lookup, since core keywords are among the most frequently visited and need no lookup at all; both branches only ever return true, so the order does not affect the result. Fixes microsoft#332
Megafia
marked this pull request as ready for review
September 13, 2026 23:32
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.
isKeywordEnabledis called per keyword per node during validation. Each call scanned every entry ofvocabularyKeywordsand ranArray.prototype.includesover its keyword list.This builds the reverse
keyword -> Set<vocabUri>index once at module load, as suggested in #332, and looks the keyword up directly.Behaviour is unchanged
Compared the old and new implementations over every possible input — all 2^14 = 16,384 subsets of active vocabularies × 63 keywords, plus the
undefinedcase:The existing suite in
src/test/vocabularies.test.tspasses unchanged.No test is added. Behaviour is identical by construction, so any new test would pass on both revisions and prove nothing about this change. Two existing tests already drive the new inner loop past its first set member:
typewith only the 2020-12 validation vocabulary active (vocabularies.test.ts:112), andformatwith onlyformat-assertionactive (:148).Performance
300,000 calls, three active 2020-12 vocabularies:
One thing beyond the issue text
The core-keyword check is hoisted above the vocabulary lookup.
$ref,$id,$schema,$defsand$commentare always enabled and among the most frequently visited keywords, so they now return without any lookup at all — that is where the 20× comes from.This is safe independently of ordering: both branches only ever
return true, so the result isA || B. (They are also disjoint in the current data — no core keyword appears in any non-core vocabulary.)Happy to drop the hoist if you would rather keep the diff to exactly what the issue describes.
Fixes #332