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

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jun 02, 2026 at 10:34:43AM +0200, Markus Armbruster wrote:
> 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.

NB tens per second, repeated across possibly 100's or even 1000's of
VMs on the single host though.

If we want an arbitrary moderately size buffer, one page feels like
a reasonable place to aim for

> 
> >                                                      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>
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|