Date: Wed, 6 Dec 2023 20:56:22 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Joe Conway <mail@joeconway.com>, Andrew Dunstan <andrew@dunslane.net>,
	Davin Shearer <davin@apache.org>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Emitting JSON to file using COPY TO
Message-ID: <20231207025622.GA3011676@nathanxps13>
References: <7a60faf6-e7f1-419d-aee6-10a78ea2fe81@joeconway.com>
 <2e7ff718-895d-83fc-46f7-be25e23b23b4@dunslane.net>
 <!bbf922c5-f571-483e-a2c6-2730b02183fe@joeconway.com>
 <1104915.1701877459@sss.pgh.pa.us>
 <19a5f9d8-bd1f-9!e51-0f5b-510c1189a8a7@dunslane.net>
 <1110690.1701880139@sss.pgh.pa.us>
 <20231206163349.GA2873889@nathanxps13>
 <20231206164439.GC2873889@nathanxps13>
 <77312d02-9325-4f14-9fe0-2746c2ee12bc@joeconway.com>
 <1136975.1701894046@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1136975.1701894046@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/20231207025622.GA3011676%40nathanxps13>
Precedence: bulk

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.

diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index 71ae53ff97..1951e93d9d 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -180,6 +180,7 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result,
 {
     char       *outputstr;
     text       *jsontext;
+    int         len;
 
     check_stack_depth();
 
@@ -223,8 +224,8 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result,
              * Don't call escape_json for a non-key if it's a valid JSON
              * number.
              */
-            if (!key_scalar && IsValidJsonNumber(outputstr, strlen(outputstr)))
-                appendStringInfoString(result, outputstr);
+            if (!key_scalar && IsValidJsonNumber(outputstr, (len = strlen(outputstr))))
+                appendBinaryStringInfo(result, outputstr, len);
             else
                 escape_json(result, outputstr);
             pfree(outputstr);

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com