EachKey panics if more than 64 keys in path

[nieksand]

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]

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

[buger]

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]

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]

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]

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]

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.

[buger]

Fixed in v1.4.0 — EachKey now handles paths with >64 key components without panicking (falls back to heap allocation when the stack array is exceeded). See: https://github.com/buger/jsonparser/releases/tag/v1.4.0