public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Chao Li <[email protected]>
Cc: jian he <[email protected]>
Cc: [email protected]
Subject: Re: Making jsonb_agg() faster
Date: Fri, 29 Aug 2025 16:09:40 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CACJufxHuE-P0s1PxiRja26ZMjKUZiCAgWa1jedswo94SFzjXMg@mail.gmail.com>
	<[email protected]>
	<CACJufxGHLYtvOAB7uA59C9Lz+rDPta5Egiii-B5opsZbw6eDDQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Chao Li <[email protected]> writes:
> A few more suggestions for pushJsonValue():
> ...
> To push WJB_BEGIN_OBJECT and WJB_END_OBJECT, we can directly call pushJsonValueScalar(), because once entering pushJsonbValue, they will meet the check of  (seq != WJB_ELEM && seq != WJB_VALUE). Directly calling pushJsonValueScalar() will saves one level of recursion.

I'm not excited about that idea, because I think it'd be quite
confusing for some of the calls in those stanzas to be to
pushJsonbValueScalar while others are to pushJsonbValue.
I don't think the recursive-push paths are particularly hot,
so giving up readability to make them faster doesn't seem like
the right tradeoff.

There's certainly room to argue that the separation between
pushJsonbValue and pushJsonbValueScalar is poorly thought out
and could be done better.  But I don't have a concrete idea
about what it could look like instead.

> And for pushJsonbValueScalar():

> -                               (*pstate)->size = 4;
> +                               ppstate->size = 4;      /* initial guess at array size */

> Can we do lazy allocation? Initially assume size = 0, only allocate memory when pushing the first element? This way, we won’t allocate memory for empty objects and arrays. 

Optimizing for the empty-array or empty-object case surely seems like
the wrong thing; how often will that apply?  I actually think that the
initial allocation could stand to be a good bit larger, maybe 64 or
256 or so entries, to reduce the number of repallocs.  I did
experiment with that a little bit and could not show any definitive
speedup on the test case I was using ... but 4 entries seems miserly
small.

			regards, tom lane

view thread (25+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Making jsonb_agg() faster
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox