Hello ChenhuiMo,

This is a really interesting optimization — doing memcpy() exponentially is
a great idea for larger counts. My main use case for repeat() is generating
large files and padding, where count is usually large, so this change helps
exactly the workload I care about.

That said, I'm not entirely sure how common this usage pattern is in
general production workloads, so I was initially unsure whether the added
complexity is worth it. Setting that aside, the performance gain for short
strings repeated a large number of times is real, so I'm fine with getting
this in.

A few things worth looking at:

1. The magic number 8 is unexplained. The naive-loop fallback threshold
seems quite low — should it be closer to a few dozen, say 64? I benchmarked
a range of thresholds and counts: the doubling approach turns out to beat
the naive loop even at very small counts (as low as 2–3), so 8 already
looks like a reasonable cutoff; raising it to 64 would likely make counts
in the 8–64 range slower, not faster. Regardless of the exact value chosen,
the constant needs a comment explaining why it was picked.

2. Because of the exponential growth, CHECK_FOR_INTERRUPTS() granularity
drops off logarithmically; the later doubling steps can copy hundreds of MB
in a single memcpy that can't be interrupted partway through. Would it help
to cap the chunk size, say at 64–100MB? I tested this: capping adds only a
handful of extra memcpy calls even for very large repeats, and the overhead
was within noise; so it looks like a cheap way to bound worst-case
interrupt latency.

3. The doubling logic itself has no comment. I think a comment there is a
must; the self-overlapping-safe memcpy trick isn't obvious at a glance.

4. It's worth adding tests that exercise count at the boundary values to
guard against regressions if this logic gets refactored later.

5. Cosmetic: add a blank line between the variable declaration and the
first statement in the new block.

Thanks

On Sun, Jul 26, 2026 at 11:00 AM ChenhuiMo <[email protected]> wrote:

> Hi all,
>
> This patch optimizes repeat(text, int).
>
> The current implementation constructs the result by copying the source text
> once per repetition.  For larger repeat counts, that leads to many small
> memcpy() calls.  This patch keeps the existing linear path for small
> repeat counts, and uses a doubling strategy for larger ones by repeatedly
> copying the already-generated prefix of the destination buffer.
>
> In local testing, this reduced total runtime in a small repeat()-focused
> microbenchmark, with the largest gains appearing in medium and large repeat
> count cases.  Small-count cases did not show obvious regressions after
> keeping the original path below a small cutoff.
>
> The patch is attached.
>
> Thanks for your time, and any feedback would be appreciated.
>
> Best regards,
> Chenhui Mo



-- 
*Jeevan Chalke*
*Senior Principal Engineer, Engineering Manager*
*Product Development*

enterprisedb.com <https://www.enterprisedb.com>

Reply via email to