search

ohler55 / ojg

Optimized JSON for Go
MIT License
857 stars 49 forks source link

Fix nested objects for Builder #96

Closed nono closed 2 years ago

nono commented 2 years ago

I have made a fix for #95. I have also added a test with fuzzing. Feel free to take only the first commit if you are not interested by the fuzzing.

ohler55 commented 2 years ago

I was offline for a bit when I created a different fix in the object-pop branch. I'll take a look at your approach before merging though.

ohler55 commented 2 years ago

I'd prefer not to defer the creation of the maps until a pop as it needlessly grows the stack. If it made sense to make a map of a preallocated size maybe using the stack to stage the map creation would make sense. I think the approach taken in the object-pop branch is simpler and more efficient although I have not run a benchmark comparison so I may be wrong..

If you don't mind I would like to cherry pick and add your fuzz tests though. It would be a nice addition.

nono commented 2 years ago

Your branch object-pop works fine for me. And you can cherry-pick my fuzz tests. Feel free to do whatever you think is best.

ohler55 commented 2 years ago

Thank you. I would be glad to consider any other PR you offer in the future.