The Wayback Machine - http://web.archive.org/web/20200519182152/https://github.com/buger/jsonparser/issues/56
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

EachKey panics if more than 64 keys in path #56

Open
nieksand opened this issue Jul 20, 2016 · 6 comments
Open

EachKey panics if more than 64 keys in path #56

nieksand opened this issue Jul 20, 2016 · 6 comments

Comments

@nieksand
Copy link

@nieksand nieksand commented Jul 20, 2016

I'm dealing with a large, flat json payload that has 100+ keys in a map.

I hit panics when using EachKey with the full list of json keys:

goroutine 1 [running]:
panic(0x4e8ac0, 0xc42000a0f0)
    /home/niek/go17/src/runtime/panic.go:500 +0x1a1
github.com/buger/jsonparser.EachKey(0xc421480000, 0x11ac, 0x11ac, 0xc420050e30, 0xc4200da000, 0xaf, 0x100, 0x0)
    /home/niek/workspace/src/github.com/buger/jsonparser/parser.go:237 +0x6a6
  ...snipped...

The problem code seems here:
https://github.com/buger/jsonparser/blob/master/parser.go#L236

If I'm reading it right, there is a limit of 64 keys per EachKey lookup before the int64 bitmask overflows.

Limit should probably be documented and code should return an error rather than panic.

@nieksand
Copy link
Author

@nieksand nieksand commented Jul 20, 2016

Correction.. only supports 63 keys given signed int64 the way bitwiseFlags is populated.

@buger
Copy link
Owner

@buger buger commented Jul 20, 2016

Yes it is, I have few ideas how to fix it without sacrificing performance.

On Wednesday, 20 July 2016, Niek Sanders notifications@github.com wrote:

Correction.. only supports 63 keys given signed int64 the way bitwiseFlags
is populated.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#56 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAA2uZbxpBvOxfZp3-ahA6iCyfMDhHoFks5qXar-gaJpZM4JQTF9
.


Sincerely yours Leonid Bugaev
http://gortool.com - test your system with real data

@nieksand
Copy link
Author

@nieksand nieksand commented Jul 21, 2016

First off, thank you for writing and sharing this library! It's been incredibly helpful for a little project I'm working on.

A wishlist item that relates to the many key lookup issue...

I wish there was a way to iterate over a map and just have a callback invoked for each key/value pair:

func(key []byte, value []byte, dataType ValueType, err error)

For a many-key scenario like mine, I can just lookup the key in a Go map to see if it's interesting to me.

Similarly, if you just want to chomp through all the keys in the map, this avoids having to determine/specify the keys you care about up front.

@nieksand
Copy link
Author

@nieksand nieksand commented Jul 21, 2016

I did a super-trashy hack implementation of my wishlist item above.

Before I invoked EachKey ~3 times per json payload to pull my data. (63 keys per call).

Now I use "WalkMap" once.

Overall performance of my app, which includes many things unrelated to json, went from 20K msgs/sec to 30k msgs/sec.

Before I was cpu-bound on json processing. Now I have free cpu and am bottlenecked elsewhere in my app.

Like I mentioned, the code is hacked garbage hence no PR.

But if you are curious it sits here: https://github.com/nieksand/jsonparser/blob/master/parser.go#L334

The actual change is here:
https://github.com/nieksand/jsonparser/blob/master/parser.go#L365

@daboyuka
Copy link
Contributor

@daboyuka daboyuka commented Jul 21, 2016

I've been thinking of prototyping this feature, as well. If I get some free
time over the next couple days I may look at your code and spin something
off of it for an actual PR.

On Jul 21, 2016 01:08, "Niek Sanders" notifications@github.com wrote:

I did a super-trashy hack implementation of my wishlist item above.

Before I invoked EachKey ~3 times per json payload to pull my data. (63
keys per call).

Now I use "WalkMap" once.

Overall performance of my app, which includes many things unrelated to
json, went from 20K msgs/sec to 30k msgs/sec.

Before I was cpu-bound on json processing. Now I have free cpu and am
bottlenecked elsewhere in my app.

Like I mentioned, the code is hacked garbage hence no PR. But if you are
curious it sits here:
https://github.com/nieksand/jsonparser/blob/master/parser.go#L334


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#56 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADvi_inTy6fvq0eF6D2AKhQ1xZb8G2BBks5qXv7qgaJpZM4JQTF9
.

@nieksand
Copy link
Author

@nieksand nieksand commented Jul 21, 2016

Very cool.

I'm thinking it might be possible to rewrite EachKey to run on top of WalkMap. That would avoid the code duplication issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.