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

Performance/Memory issues with rdl.Validate when FullValidation enabled #26

Closed
evantorrie opened this issue Apr 24, 2017 · 5 comments
Closed

Comments

@evantorrie
Copy link
Contributor

evantorrie commented Apr 24, 2017

The current go implementation of rdl.Validate calls NewTypeRegistry(schema) on every invocation. For a complex schema, this is a costly operation.

checker.registry = NewTypeRegistry(schema)

This becomes apparent in go when unmarshalling large objects with nested structs containing a lot of "derived-from-String" types.

In tests, I've found that code which would just cache the TypeRegistry per schema when calling schema.Build() and then use that in a rdl.Validate2() style call which can accept a preconstructed TypeRegistry gives a close to 10x improvement on JSON unmarshalling speed.

@boynton
Copy link
Collaborator

boynton commented Apr 24, 2017

Thanks, good find. Will fix that. Or... you can if you want :-)

@evantorrie
Copy link
Contributor Author

I have a fix which involves exposing a Validator object -- and then having the code generator generate an instance of that at schema build time. I'll submit a PR, but I don't know enough details about the implementations in other languages to know whether it's the best way to solve this issue.

@boynton
Copy link
Collaborator

boynton commented Apr 24, 2017

Well, Fixing it behind the scenes would be best, it is just a performance optimization for the Go library, other libraries have different ways to deal with it, so don't worry about those just yet. If you want to just send me a sketch (or pointer to your changes) I can integrate it, I just thought you were closer to it, having done the performance comparison already.

@evantorrie
Copy link
Contributor Author

I changed it to use a transparent cache - no change to the exposed API.

Take a look at #29

@boynton
Copy link
Collaborator

boynton commented May 14, 2017

Fixed in Release 1.4.13, PR #29

@boynton boynton closed this as completed May 14, 2017
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

No branches or pull requests

2 participants