Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Support event driven parsing. #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

canatella
Copy link

This pull requests adds "SAX" like event driven parsing to json11. When parsing lots of objects, it allows having access to them without waiting for the whole parsing to finish. It should also limit memory usage.

This is a tentative request, it still needs work but I wanted to know if you would be open to such a feature and how you would envision it. I still need to

  • write tests,
  • avoid keeping all data in memory.

Also I'm not an expert in C++ so do not hesitate to point me to the problems in my code.

@smarx
Copy link

smarx commented Dec 2, 2016

Automated message from Dropbox CLA bot

@canatella, thanks for the pull request! It looks like you haven't yet signed the Dropbox CLA. Please sign it here.

@artwyman
Copy link
Contributor

Hi @canatella. Trying to get caught up on PR's during the holiday lull.
I'm not opposed to supporting event-based parsing in theory (though I wonder if @j4cbo has thoughts as primary author). The current strategy of mixing it into the standard parser does seem not to get most of the benefit, due to keeping everything in memory as you mention. In most cases there wouldn't really be a benefit to using the events, vs. finishing the parse and then examining the resulting object. You could get earlier results on a streaming parse, but that should only be relevant for long streams, in which case the memory size would be an issue. My guess is that if you wanted the full benefit of this style of parser, you'd want it to be the primary parse mechanism, then have the "standard" parse be built as an event handler.

Copy link
Contributor

@artwyman artwyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments above, but I'm going to use the code-review flow for this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants