Re: [Qemu-devel] [PATCH 4/4] qjson: surprise, allocating 6 QObjects per token is expensive

[Eric Blake]

On 11/20/2015 02:04 AM, Paolo Bonzini wrote:
> Replace the contents of the tokens GQueue with a simple struct.  This cuts
> the amount of memory allocated by tests/check-qjson from ~500MB to ~20MB,
> and the execution time from 600ms to 80ms on my laptop.  Still a lot (some
> could be saved by using an intrusive list, such as QSIMPLEQ, instead of
> the GQueue), but the savings are already massive and the right thing to
> do would probably be to get rid of json-streamer completely.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  include/qapi/qmp/json-streamer.h |  7 ++++
>  qobject/json-parser.c            | 81 
> +++++++++++++++++++---------------------
>  qobject/json-streamer.c          | 19 ++++------
>  3 files changed, 53 insertions(+), 54 deletions(-)
> 

> @@ -54,15 +50,16 @@ static void json_message_process_token(JSONLexer *lexer, 
> GString *input, JSONTok
>          }
>      }
>  
> -    dict = qdict_new();
> -    qdict_put(dict, "type", qint_from_int(type));
> -    qdict_put(dict, "token", qstring_from_str(input->str));
> -    qdict_put(dict, "x", qint_from_int(x));
> -    qdict_put(dict, "y", qint_from_int(y));
> +    token = g_malloc(sizeof(JSONToken) + input->len + 1);
> +    token->type = type;
> +    memcpy(token->str, input->str, input->len + 1);
> +    token->str[input->len] = 0;

Looks like you are writing the last byte twice.  Either the +1 in the
memcpy() always copies a NUL byte and we don't need the second
assignment, or you should drop the +1.

Otherwise, looks like a sane replacement that saves a lot of memory.

Reviewed-by: Eric Blake <address@hidden>

Are we hoping to get this in 2.5 because it fixes the memory hog bug, or
are we considering that it is not a regression from 2.4 and therefore
something that should wait for 2.6?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature