| From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
|---|---|
| To: | Joe Conway <mail(at)joeconway(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Davin Shearer <davin(at)apache(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | micro-optimizing json.c |
| Date: | 2023-12-07 23:12:51 |
| Message-ID: | 20231207231251.GB3359478@nathanxps13 |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Moving this to a new thread...
On Thu, Dec 07, 2023 at 07:15:28AM -0500, Joe Conway wrote:
> On 12/6/23 21:56, Nathan Bossart wrote:
>> On Wed, Dec 06, 2023 at 03:20:46PM -0500, Tom Lane wrote:
>> > If Nathan's perf results hold up elsewhere, it seems like some
>> > micro-optimization around the text-pushing (appendStringInfoString)
>> > might be more useful than caching. The 7% spent in cache lookups
>> > could be worth going after later, but it's not the top of the list.
>>
>> Hah, it turns out my benchmark of 110M integers really stresses the
>> JSONTYPE_NUMERIC path in datum_to_json_internal(). That particular path
>> calls strlen() twice: once for IsValidJsonNumber(), and once in
>> appendStringInfoString(). If I save the result from IsValidJsonNumber()
>> and give it to appendBinaryStringInfo() instead, the COPY goes ~8% faster.
>> It's probably worth giving datum_to_json_internal() a closer look in a new
>> thread.
>
> Yep, after looking through that code I was going to make the point that your
> 11 integer test was over indexing on that one type. I am sure there are
> other micro-optimizations to be made here, but I also think that it is
> outside the scope of the COPY TO JSON patch.
Here's a patch that removes a couple of strlen() calls that showed up
prominently in perf for a COPY TO (FORMAT json) on 110M integers. On my
laptop, I see a 20% speedup from ~23.6s to ~18.9s for this test.
I plan to test the other types as well, and I'd also like to look into the
caching mentioned above if/when COPY TO (FORMAT json) is committed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-avoid-some-strlen-calls-in-json.c.patch | text/x-diff | 2.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jeff Davis | 2023-12-07 23:43:47 | Re: micro-optimizing json.c |
| Previous Message | Nathan Bossart | 2023-12-07 23:02:15 | Re: postgres_fdw test timeouts |