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

first crack at extracting values from arrays by giving an index as a … #63

Conversation

dparadise28
Copy link

@dparadise28 dparadise28 commented Aug 20, 2016

Description: What this PR does
goal is to extract values from arrays nested in deeper structures and values from within those array values

an example is provided in the added test

Benchmark before change:

docker run -v `pwd`:/go/src/github.com/buger/jsonparser -i -t jsonparser go test  -test.benchmem -bench JsonParser ./benchmark/  -benchtime 5s -v
testing: warning: no tests to run
PASS
BenchmarkJsonParserLarge-2                         50000            182450 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserMedium-2                       200000             33686 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualMedium-2          500000             18086 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserSmall-2                       2000000              3361 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualSmall-2          5000000              1905 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyStructSmall-2          3000000              2776 ns/op             176 B/op          7 allocs/op
ok      github.com/buger/jsonparser/benchmark   59.980s

**Benchmark after change**:
docker run -v `pwd`:/go/src/github.com/buger/jsonparser -i -t jsonparser go test  -test.benchmem -bench JsonParser ./benchmark/  -benchtime 5s -v
testing: warning: no tests to run
PASS
BenchmarkJsonParserLarge-2                         50000            183814 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserMedium-2                       200000             34856 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualMedium-2          500000             17943 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserSmall-2                       2000000              3403 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyManualSmall-2          5000000              1877 ns/op               0 B/op          0 allocs/op
BenchmarkJsonParserEachKeyStructSmall-2          3000000              2786 ns/op             176 B/op          7 allocs/op
ok      github.com/buger/jsonparser/benchmark   60.314s

@buger
Copy link
Owner

buger commented Aug 20, 2016

Looks really cool!

Did you tried to re-use existing ArrayEach method, instead of embedding own parser?

@dparadise28
Copy link
Author

dparadise28 commented Aug 20, 2016

Thanks! I'm glad you like it. I'm working on something that will be using the hard work you and all the contributors put into your parser and I came across something i though would be useful so figured I'd see if i could contribute.

Reusing ArrayEach was actually my first instinct and then i decided to go with somewhat duplicate parsing logic in an effort to be a bit more efficient and not iterate through part of the slice twice given you cant break when your using array each. But given the code is definitely more readable and there is greater reuse in this approach I think your right on point. And maybe we can think of a way to break out of ArrayEach aft something is triggered in order to optimize the impl a bit further.

@rami-dabain
Copy link

for breaking from ArrayEach: #58 (comment) though haven't tried it yet

@buger
Copy link
Owner

buger commented Dec 16, 2016

Nailed it #77 !

Thank you for the first version! I had to completely rewrite it, and add support for EachKey.

@buger buger closed this Dec 16, 2016
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