Re: [PATCH] qobject/json-writer: preallocate output buffer

[Markus Armbruster <armbru@redhat.com>]

Bin Guo <guobin@linux.alibaba.com> writes:

> json_writer_new() creates the output GString with g_string_new(NULL),
> which starts at the GLib default of 64 bytes.  Serializing typical
> QMP responses then requires multiple reallocations as the buffer
> grows -- for query-qmp-schema the GString is reallocated 12+ times.

That's an extreme case.  Most responses are *much* smaller.  Still,
starting with a larger buffer makes sense.

> Preallocate JSON_WRITER_INITIAL_SIZE (4096) bytes.  This covers
> most QMP responses without any reallocation.  4096 is one page on
> most systems, which is efficient for the allocator.

I doubt "one page" matters.  How many QMP commands get executed in
practice?  A couple of hundred during startup, then tens per second?
Probably less than that.

>                                                      The JSONWriter
> is a short-lived object so the preallocation does not accumulate.
>
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  qobject/json-writer.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/qobject/json-writer.c b/qobject/json-writer.c
> index aac2c6ab71..fb3f3f3e3c 100644
> --- a/qobject/json-writer.c
> +++ b/qobject/json-writer.c
> @@ -24,13 +24,16 @@ struct JSONWriter {
>      GByteArray *container_is_array;
>  };
>  
> +/* Covers most QMP responses without reallocation (one page) */

Covering most responses matters, one page does not.  Suggest

   /* Should cover most QMP responses without reallocation */

> +#define JSON_WRITER_INITIAL_SIZE  4096
> +
>  JSONWriter *json_writer_new(bool pretty)
>  {
>      JSONWriter *writer = g_new(JSONWriter, 1);
>  
>      writer->pretty = pretty;
>      writer->need_comma = false;
> -    writer->contents = g_string_new(NULL);
> +    writer->contents = g_string_sized_new(JSON_WRITER_INITIAL_SIZE);
>      writer->container_is_array = g_byte_array_new();
>      return writer;
>  }

Consider tweaking the commit message and the comment to address my
remarks.

Reviewed-by: Markus Armbruster <armbru@redhat.com>