brandur / json_schema

A JSON Schema V4 and Hyperschema V4 parser and validator.
MIT License
230 stars 45 forks source link

precalculate valid Ruby types to speed up validation #94

Closed breunigs closed 6 years ago

breunigs commented 6 years ago

Right now, validate_type always rebuilds the array of valid Ruby types from the list of valid JSON schema types. Unsurprisingly caching helps (~10% speedup):

8,941 8,869 8,639 8,713 9,030 vanilla
8,026 7,966 7,914 7,951 8,030 pre-calculate types

The patch is not clearing the cache if the types get re-assigned, although it would be easy enough to add. In general I am unsure if this is a worthwhile tradeoff – the previous patches were mostly "no downisdes at all", but this one introduces caching.

brandur commented 6 years ago

Thanks, and thanks for including the numbers! Nice work.

The patch is not clearing the cache if the types get re-assigned, although it would be easy enough to add. In general I am unsure if this is a worthwhile tradeoff – the previous patches were mostly "no downisdes at all", but this one introduces caching.

In terms of the tradeoff, do you think there's a particular downside to allowing that cache to be cleared on assignment?

I'm a little on the fence for this one — I think that reassigning your types after initialization is probably a relatively rare operation, but on the other hand, it wouldn't be that surprising if someone was doing it somewhere, so it may be best to try and keep this working.

breunigs commented 6 years ago

regarding clearing the cache: No, it should be simple enough to reset the cache. I'll have a look at finding a clean implementation without blowing up schema_attr.

regarding the nil check: I just checked, and the only way would be to manually/explicitly set it to nil through something like schema.type = nil. This is equally unlikely as the cache clear. I'll add it back and see if this actually changes anything in terms of performance (I doubt it). If it does, I'll see if I can add a writer method that makes it impossible to set it nil. In any case, I think ease of use values higher than minuscule perf gains.

brandur commented 6 years ago

Nice work on the cache clearing scheme. Thanks!

brandur commented 6 years ago

Released as 0.18.0.