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

add caching to JsonRef.fromURI #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

raimohanska
Copy link

I found out that by caching the results of JsonRef.fromURI, I can get almost 10x performance boost in the JSON Schema validation in my application. Maybe consider including the optimization into json-schema-core?

raimohanska added a commit to Opetushallitus/koski that referenced this pull request May 10, 2016
@huggsboson
Copy link
Member

seems super useful. I'm a bit hesitant about the static, I'm looking to see if there's a place that does all this resolving where we can put a cache instead of using a static.

@raimohanska
Copy link
Author

Sounds great! A global static cache may in fact cause a mem leak in some cases.

@huggsboson
Copy link
Member

I probably won't get time to do it. Any chance you can take a look? Seems like a good change and I don't want to lose it because I'm swamped.

@Capstan
Copy link
Contributor

Capstan commented Oct 29, 2019

Looking at the calls to fromURI, it is called in a bunch of places just within the java-json-tools repos, and likely external libraries. I think the best we could do within compatibility is provide a public static field which fromURI called, which allowed for replacing with an alternate cache mechanism, so clients could fiddle with the cache parameters. But it'd still be global, and clients might fight over it.

A more invasive deprecation of the static fromURI and replacing it with a URI factory that we then have to provide to all the places where fromURI is currently called might be suitable for a larger contribution (and a version bump).

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.

3 participants