Improve validators through a trait index on model #412
Conversation
This file contains 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
kstich
requested changes
May 4, 2020
This commit is a fairly large update to validators because there's a new super useful feature on Model where you can now get a Set of shapes that have a specific trait (and the value is cached). This means that a large majority of validators that utilized Trait.flatMapStream can now be much more efficiently written by iterating over the shapes that actually have the trait directly. Lots of validators were updated to take advantage of this and be more efficient (particularly HTTP validators and trait conflict validators). I also noticed that the TraitValueValidator was validating the prelude model, and that was pretty expensive. I updated this validator to only validate the prelude when a specific metadata property is set. Then, in unit tests, I added a test that ensures that the validator validates the prelude so that we can be sure that changes made to the prelude are valid. Because of this change and the trait indexing, JMH shows that the time to load and validate a model was nearly cut in half (from about 2ms to 1ms on my old laptop). I noticed that the UnstableFeatureValidator was still being used on document shapes even though the spec was updated to remove the disclaimer that document shapes are unstable, so I removed this validator.
This commit rewrites a lot of NodeValidationVisitors to now emit events from validation plugins and then allow the visitor to associate the right context, shape, and severity. This allows the events emitted to the visitor to contain a more specific source location than N/A.
kstich
approved these changes
May 4, 2020
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.
This commit is a fairly large update to validators because there's a new
super useful feature on Model where you can now get a Set of shapes that
have a specific trait (and the value is cached). This means that a large
majority of validators that utilized Trait.flatMapStream can now be much
more efficiently written by iterating over the shapes that actually have
the trait directly. Lots of validators were updated to take advantage of
this and be more efficient (particularly HTTP validators and trait
conflict validators).
I also noticed that the TraitValueValidator was validating the prelude
model, and that was pretty expensive. I updated this validator to only
validate the prelude when a specific metadata property is set. Then, in
unit tests, I added a test that ensures that the validator validates the
prelude so that we can be sure that changes made to the prelude are
valid. Because of this change and the trait indexing, JMH shows that the
time to load and validate a model was nearly cut in half
(from about 2ms to 1ms on my old laptop).
I noticed that the UnstableFeatureValidator was still being used on
document shapes even though the spec was updated to remove the
disclaimer that document shapes are unstable, so I removed this
validator.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.