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

Use streaming JSON parser for combining JSON results #21

Open
chrisdone opened this issue Jun 15, 2019 · 3 comments
Open

Use streaming JSON parser for combining JSON results #21

chrisdone opened this issue Jun 15, 2019 · 3 comments

Comments

@chrisdone
Copy link

Currently we're using aeson's decode to parse into Value structures, lookup the data field, then union the HashMaps, which is convenient but less efficient than if we parsed the boundaries of the object using the attoparsec combinators, located the data key boundaries, and then just concatenated the bytestrings together at the right point as a Builder (the EncJSON uses this). Here is an example of parsing directly.

@jberryman
Copy link
Collaborator

jberryman commented Aug 12, 2019

I just feel icky doing performance work when we don't have benchmarks. It's first of all hard to tell if the effort is worth it, and second we don't get to feel good about any big gains we've attained.

@tirumaraiselvan do you have any thoughts on benchmarking as it relates to this feature?

also relevant to #30

@jberryman
Copy link
Collaborator

I can take this one.

@tirumaraiselvan
Copy link
Owner

Yeah, these are optimizations that we better do more systematically. So let's put this on hold.

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

No branches or pull requests

3 participants