Re: [PATCH] Speed up repeat() for larger counts

From: Jan Nidzwetzki <jan(at)planetscale(dot)com>
To: ChenhuiMo <chenhuimo(dot)mch(at)qq(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Subject: Re: [PATCH] Speed up repeat() for larger counts
Date: 2026-09-14 10:13:46
Message-ID: 01612e7c-ac8c-45ad-9739-c13675cfd3e5@planetscale.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Chenhui,

Thanks for the new version and for splitting the patch set. It applies
to the current master branch, and check-world passes.

On 12.09.26 15:12, ChenhuiMo wrote:

[...]

> Here the non-temporal threshold is 12.625 MiB, so the first tested cap
> at or above it is 16 MiB. This also matches the earlier measurements,
> where cap16m was often the best choice for the larger-output cases.
>
> So the glibc non-temporal threshold appears to correlate well with the
> preferred cap on both of my x86 systems.

That settles it for me. Collecting the three x86 systems measured so far:

system non-temporal threshold first cap at or above it
J5005 1 MiB cap1m
i5-13500H 1.3125 MiB cap2m
i9-12900H 12.625 MiB cap16m

In every case, the preferred cap is the first one at or above the
threshold. So the 64 KB to 16 MB spread we started with is a glibc
setting with three different values, not a hardware property.

Review
======
In patch 2, CHECK_FOR_INTERRUPTS() is called after memset(). Usually,
CHECK_FOR_INTERRUPTS() is called periodically to check whether a
long-running task has to be aborted. After memset() returns, the actual
heavy work is already done. Not sure if we actually need the check.

In addition, I recommend a short comment for the new 'if (slen == 1)'
path that describes that memset() is faster than the loop below.

strings.sql offers only partial test coverage for the added conditions.
I recommend adding at least these three tests to the existing ones to
test the early return and the memset() fast path:

SELECT repeat('', 10);
SELECT repeat('x', 0);
SELECT repeat('x', 4);

Additional comments
===================
Your new patch set has a different author than the previous ones. Please
double-check that it contains the actual name you want visible in the
commit.

When you generate a new patch set, it is common to version it to make it
easier for others to understand which version is most recent. You can do
this by specifying the "-v" option when exporting the patch. For
example, "git format-patch -v3 -2" generates version 3 of the patch
series consisting of the last two commits.

Best regards
Jan

--
Jan Nidzwetzki
PlanetScale Postgres Core Team

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Vaibhav Dalvi 2026-09-14 10:30:58 Re: Proposal: INSERT ... BY NAME
Previous Message Osama Abdul Qader 2026-09-14 09:59:51 Re: Fix unnecessary shared memory page allocation in CalculateShmemSize()