Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplifies how evaluated properties and array items are tracked #790

Merged
merged 7 commits into from
Jun 5, 2023

Conversation

fdutton
Copy link
Contributor

@fdutton fdutton commented May 29, 2023

Should improve performance.

Resolves #721

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2023

Codecov Report

Merging #790 (aa8b080) into master (3c9d1d4) will decrease coverage by 0.49%.
The diff coverage is 82.91%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff              @@
##             master     #790      +/-   ##
============================================
- Coverage     75.66%   75.18%   -0.49%     
- Complexity     1212     1242      +30     
============================================
  Files           122      122              
  Lines          4118     4163      +45     
  Branches        792      801       +9     
============================================
+ Hits           3116     3130      +14     
- Misses          679      706      +27     
- Partials        323      327       +4     
Impacted Files Coverage Δ
...a/com/networknt/schema/SchemaValidatorsConfig.java 65.18% <54.09%> (-7.86%) ⬇️
...in/java/com/networknt/schema/CollectorContext.java 74.62% <66.66%> (-9.82%) ⬇️
...main/java/com/networknt/schema/OneOfValidator.java 74.60% <84.21%> (+0.36%) ⬆️
...c/main/java/com/networknt/schema/RefValidator.java 90.00% <86.95%> (-0.13%) ⬇️
...om/networknt/schema/UnevaluatedItemsValidator.java 89.13% <91.42%> (-4.21%) ⬇️
...tworknt/schema/UnevaluatedPropertiesValidator.java 89.13% <92.10%> (-5.87%) ⬇️
...main/java/com/networknt/schema/AnyOfValidator.java 91.54% <93.33%> (+1.13%) ⬆️
src/main/java/com/networknt/schema/JsonSchema.java 88.88% <96.42%> (+0.99%) ⬆️
...main/java/com/networknt/schema/AllOfValidator.java 92.30% <100.00%> (-1.45%) ⬇️
...rc/main/java/com/networknt/schema/IfValidator.java 80.43% <100.00%> (-6.71%) ⬇️
... and 6 more

@stevehu
Copy link
Contributor

stevehu commented May 29, 2023

I tested with the json-schema-validdator-perftest between issue721 and master. The performance is almost identical on my computer. Do I need to turn on a config flag? Thanks.

@fdutton
Copy link
Contributor Author

fdutton commented May 29, 2023

@stevehu No, there is no config flag yet. In the comment I left on #721, I attempted to explain that I am not seeing the issue.

Since then, I managed to find a somewhat complicated schema and generated a 75MB JSON file from it. I ran it in json-schema-validator-perftest where is averaged 766 ms and 1000ms for worst time. The schema does not use unevaluatedProperties or unevaluatedItems so I'm adding those to see the effect.

@fdutton
Copy link
Contributor Author

fdutton commented May 29, 2023

unevaluatedProperties adds 7% overhead using this branch.

@fdutton fdutton marked this pull request as draft May 29, 2023 21:20
Faron Dutton added 5 commits June 3, 2023 16:00
# Conflicts:
#	src/test/java/com/networknt/schema/suite/TestCase.java
#	src/test/java/com/networknt/schema/suite/TestSpec.java
# Conflicts:
#	src/test/java/com/networknt/schema/AbstractJsonSchemaTestSuite.java
@fdutton fdutton marked this pull request as ready for review June 4, 2023 18:47
@fdutton
Copy link
Contributor Author

fdutton commented Jun 4, 2023

@stevehu Please merge this change so I do not have to keep resolving conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Significant performance regression in 1.0.80
3 participants