public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Tom Lane <[email protected]>
Cc: jian he <[email protected]>
Cc: [email protected]
Subject: Re: Making jsonb_agg() faster
Date: Mon, 25 Aug 2025 17:19:34 +0800
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]>
> On Aug 23, 2025, at 03:11, Tom Lane <[email protected]> wrote:
>
>
> v2-0001 takes care of that, and also adopts your suggestion in [1]
> about not using two calls of pushJsonbValueScalar where one would do.
> I also did a bit more micro-optimization in appendKey, appendValue,
> appendElement to avoid redundant copying, because perf testing showed
> that appendElement is still a hot-spot for jsonb_agg. Patches 0002
> and 0003 are unchanged.
>
A small comment. In most of places, JsonbInState is initialized as “JsonbInState ps = {0};”, which is the preferred C style. There are some other places still using memset() to zero out JsonbInState variables. Maybe we can change all memset() usages to “{0}”.
I did a search and found out all memset to change:
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 0c964052958..9ca501a7a6e 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -240,10 +240,10 @@ static inline Datum
jsonb_from_cstring(char *json, int len, bool unique_keys, Node *escontext)
{
JsonLexContext lex;
- JsonbInState state;
+ JsonbInState state = {0};
JsonSemAction sem;
- memset(&state, 0, sizeof(state));
+ //memset(&state, 0, sizeof(state));
memset(&sem, 0, sizeof(sem));
makeJsonLexContextCstringLen(&lex, json, len, GetDatabaseEncoding(), true);
@@ -1147,9 +1147,9 @@ to_jsonb(PG_FUNCTION_ARGS)
Datum
datum_to_jsonb(Datum val, JsonTypeCategory tcategory, Oid outfuncoid)
{
- JsonbInState result;
+ JsonbInState result = {0};
- memset(&result, 0, sizeof(JsonbInState));
+ //memset(&result, 0, sizeof(JsonbInState));
datum_to_jsonb_internal(val, false, &result, tcategory, outfuncoid,
false);
@@ -1162,7 +1162,7 @@ jsonb_build_object_worker(int nargs, const Datum *args, const bool *nulls, const
bool absent_on_null, bool unique_keys)
{
int i;
- JsonbInState result;
+ JsonbInState result = {0};
if (nargs % 2 != 0)
ereport(ERROR,
@@ -1172,7 +1172,7 @@ jsonb_build_object_worker(int nargs, const Datum *args, const bool *nulls, const
errhint("The arguments of %s must consist of alternating keys and values.",
"jsonb_build_object()")));
- memset(&result, 0, sizeof(JsonbInState));
+ //memset(&result, 0, sizeof(JsonbInState));
pushJsonbValue(&result, WJB_BEGIN_OBJECT, NULL);
result.parseState->unique_keys = unique_keys;
@@ -1232,9 +1232,9 @@ jsonb_build_object(PG_FUNCTION_ARGS)
Datum
jsonb_build_object_noargs(PG_FUNCTION_ARGS)
{
- JsonbInState result;
+ JsonbInState result = {0};
- memset(&result, 0, sizeof(JsonbInState));
+ //memset(&result, 0, sizeof(JsonbInState));
pushJsonbValue(&result, WJB_BEGIN_OBJECT, NULL);
pushJsonbValue(&result, WJB_END_OBJECT, NULL);
@@ -1293,9 +1293,9 @@ jsonb_build_array(PG_FUNCTION_ARGS)
Datum
jsonb_build_array_noargs(PG_FUNCTION_ARGS)
{
- JsonbInState result;
+ JsonbInState result = {0};
- memset(&result, 0, sizeof(JsonbInState));
+ //memset(&result, 0, sizeof(JsonbInState));
pushJsonbValue(&result, WJB_BEGIN_ARRAY, NULL);
pushJsonbValue(&result, WJB_END_ARRAY, NULL);
@@ -1321,9 +1321,9 @@ jsonb_object(PG_FUNCTION_ARGS)
int in_count,
count,
i;
- JsonbInState result;
+ JsonbInState result = {0};
- memset(&result, 0, sizeof(JsonbInState));
+ //memset(&result, 0, sizeof(JsonbInState));
pushJsonbValue(&result, WJB_BEGIN_OBJECT, NULL);
@@ -1425,9 +1425,9 @@ jsonb_object_two_arg(PG_FUNCTION_ARGS)
int key_count,
val_count,
i;
- JsonbInState result;
+ JsonbInState result = {0};
- memset(&result, 0, sizeof(JsonbInState));
+ //memset(&result, 0, sizeof(JsonbInState));
pushJsonbValue(&result, WJB_BEGIN_OBJECT, NULL);
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index f0d2378a8cc..4305623b979 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2872,7 +2872,7 @@ executeKeyValueMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
{
JsonBaseObjectInfo baseObject;
JsonbValue obj;
- JsonbInState ps;
+ JsonbInState ps = {0};
Jsonb *jsonb;
if (tok != WJB_KEY)
@@ -2886,7 +2886,7 @@ executeKeyValueMethod(JsonPathExecContext *cxt, JsonPathItem *jsp,
tok = JsonbIteratorNext(&it, &val, true);
Assert(tok == WJB_VALUE);
- memset(&ps, 0, sizeof(ps));
+ //memset(&ps, 0, sizeof(ps));
pushJsonbValue(&ps, WJB_BEGIN_OBJECT, NULL);
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
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