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

using jsoniter #271

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

using jsoniter #271

wants to merge 1 commit into from

Conversation

aadij
Copy link

@aadij aadij commented Oct 3, 2019

  • using json-iterator instead of encoding/json for better performance

@johandorland
Copy link
Collaborator

Do you have any number on how much faster this is going to be? From my experience JSON schema parsing itself is already quite heavy (and there are many areas that can easily be optimized). I'm not sure how much of an impact JSON encoding/decoding makes overall (especially when you use json-iterator in a compatibility mode). To me this feels like premature optimization and I somewhat like not having a dependency on a specific JSON parser, although feel free to change my mind.

@aadij
Copy link
Author

aadij commented Oct 4, 2019

validator-benchmarks (master) > go test -bench=BenchmarkX -benchmem validator_test.go 
goos: darwin
goarch: amd64
BenchmarkXeipuu-12                   200           7055320 ns/op         4604919 B/op      63053 allocs/op
BenchmarkXeipuuFork-12               200           6139997 ns/op         3803831 B/op      59141 allocs/op
PASS
ok      command-line-arguments  4.072s

Benchmark results using fork of validator-benchmarks

~13% saving in execution time and ~17% saving in memory usage per operation even in compatibility mode.

@aadij
Copy link
Author

aadij commented Oct 11, 2019

@johandorland are these improvements significant enough?

@johandorland
Copy link
Collaborator

I don't think the JSON schema test suite is particularly useful as its main purpose is for a validator to test their compliance to the spec . The schemas and documents in that test suite are all very small. If on more realistic schemas the numbers look similar it might be worth at least making JSON implementation pluggable.

@rainingmaster
Copy link

It well know that json-iterator is more faster than std json from here, and this project is also use in many other golang project, such as etcd, gin

@johandorland
Copy link
Collaborator

Etcd is an application and gin is a framework, which have more leniancy when it comes to making oppinionated choices. gojsonschema will often just be one of the libraries part of an application and I don't like having a dependency on a JSON parser.

After sleeping on this I think the root of this problem can be solved in a different manner. It's already possible for the user to directly pass parsed JSON using NewRawLoader. It's currently undocumented and tricky to use, but for these kind of use cases I think it makes perfect sense. The only thing I'm not sure about is if and what modifications need to be done to gojsonschema so it's less picky about it's raw input types. It doesn't seem like jsoniter use the exact same types as the standard library does.

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.

None yet

3 participants