Skip to content

Json Print Stream#89

Closed
iMobs wants to merge 6 commits intoDaveGamble:masterfrom
iMobs:json_stream
Closed

Json Print Stream#89
iMobs wants to merge 6 commits intoDaveGamble:masterfrom
iMobs:json_stream

Conversation

@iMobs
Copy link

@iMobs iMobs commented Jan 8, 2017

This method of serializing cJSON objects does not use malloc or free and instead writes to a buffer on the stack which once full will be sent to a user provided callback. In the callback a user can for example write to a file or socket as each chunk of data is ready without needing the entire formatted string. Internally it is formatting the data in the exact same way as the original print functions.

The current method of serializing cJSON objects is pretty memory intensive with multiple calls to malloc/free and gets greedy for larger nested cJSON objects. Using a printbuffer can be wasteful as it will allocate more memory than necessary if the estimated space is too small. This alternative does not use the heap and only requires a small increase in stack size and the user can decide how they want to store the string or if they need to if all they're doing is immediately writing it out.

There is still some documentation that I would like to add in, but the code itself is ready for use.

Ian Mobley added 5 commits January 4, 2017 15:54
Just jotting down the first steps in streaming a serialized cJSON
object. The point being that instead of allocating and/or reallocating
a potentially large buffer, the serialized data just gets periodically
written out. It's similar to the printbuffer, but instead of
reallocating when the buffer is full, it just flushes the current string
out to a user provided callback.
Still need to handle strings larger than the stream buffer and some kind
of error handling.

Still need to fix tab depth for objects and arrays.

Also need to consider adding a function for simply adding characters to
the stream since that would be a simpler to use for the array and object
functions.
Fleshed out the stream_string to deal with escaped values

Fixed tabs/spaces from vim default
If the stream->error value is non zero, most functions will try to
return immediately. Currently the error is only set from the user
provided callback, but can easily be modified elsewhere in future use.
@FSMaxB
Copy link
Collaborator

FSMaxB commented Jan 12, 2017

I haven't had time to fully review this yet. The idea of using streaming for parsing/printing sounds compelling though.

Nevertheless, the solution cannot be a complete duplication of the existing code, so if this is going to be implemented, it should be done in a way that allows reusing the existing code for printing/parsing.

Also please discuss the technical details before going of and starting to write new code.

I also think that #45 should be sorted out before a change like this is attempted.

@iMobs
Copy link
Author

iMobs commented Jan 12, 2017

So one of the things I was thinking about was what if this was the primary way of formatting output and all the cJSON_Print functions instead used internal callbacks to allocate and copy to their buffer or prebuffer. Or it could be run once where it simply gets the buffer size necessary but doesn't copy, then allocates and runs again similar to using size_t s = snprintf(NULL, 0, "%s: %d", "Example, 1") to get the length of a formatted string. Then the old print functions that really on malloc and free wouldn't be necessary.

For example, cJSON_Print can be redone like this and only call malloc once.

struct print_data
{
    char *buffer;
    size_t offset;
};

static int print_callback(const char *str, size_t length, void *data)
{
    struct print_data *print_data = (struct print_data *)data;

    strncpy(print_data->buffer + print_data->offset, str, length);
    print_data->offset += length;

    return 0;
}

/* Render a cJSON item/entity/structure to text. */
char *cJSON_Print(const cJSON *item)
{
    cJSON_Stream stream;
    struct print_data print_data;
    size_t size;

    cJSON_StreamInit(&stream);
    stream.fmt = true;

    size = cJSON_PrintStream(&stream, item) + 1;
    print_data.buffer = cJSON_malloc(size);

    if (print_data.buffer)
    {
        memset(print_data.buffer, 0, size);
        print_data.offset = 0;

        stream.cb = print_callback;
        stream.cb_data = &print_data;

        cJSON_PrintStream(&stream, item);
    }

    return print_data.buffer;
}

void *cb_data;

/* Whether or not to format the JSON output */
int fmt;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this doesn't belong in the stream but should remain a parameter to the print function.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Jan 17, 2017

One thing I thought about is to replace every call to cJSON_strdup, sprintf and similar ones that print data, with a function pointer that can be switched. In normal mode this would actually call cJSON_strdup ... or things like memcpy.

But this could potentially have a really big impact on performance, so some benchmarks have to be made to determine if this is a viable option.

Also I don't like the idea of running the print twice, one time for determining the size.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Jan 17, 2017

But I guess the only way to really know the actual performance impact is to try the different approaches with lots of datasets and compare the results.

Also keep in mind that the approach that is chosen should be extendable to parsing as well.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Mar 19, 2017

I'm closing this. I have some ideas on how streaming can be implemented, I'll open a separate issue. But it has a low priority right now.

@FSMaxB FSMaxB closed this Mar 19, 2017
@FSMaxB FSMaxB mentioned this pull request Mar 19, 2017
@iMobs iMobs deleted the json_stream branch April 17, 2017 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants