Optimize str_repeat

[divinity76]

The memmove got me thinking "can src and dst actually overlap?", and it turns out no:

• e always points to populated data + 1
• l may reach the end of populated data, but will never exceed it

memcpy is safe here, and faster (unless the compiler already spotted it in an optimization pass and replaced it internally, entirely possible)

[Copilot]

Pull request overview

This pull request optimizes PHP_FUNCTION(str_repeat) in ext/standard/string.c by replacing a memmove() with memcpy() in the exponential self-copy loop used to build the repeated string, based on the non-overlap guarantee of the source and destination ranges in that loop.

Changes:

• Replace memmove(e, s, l) with memcpy(e, s, l) in the loop that expands the already-copied prefix into the remaining buffer.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[LamentXU123]

Uh, from my knowledge the only difference between them is memmove check if s and e is the same, and is slower than memcpy.

In this case, I can't imagine any edge-cases because the two address shouldn't be the same. So this is sensible. But I don't know whether we want minor optimizations, because compilers are smart, and I don't sure if there is more difference between them.

David is knowledgeable about UNIX stuff. Maybe he could decide this. cc @devnexen :)

[LamentXU123]

From my perspective. But please wait for other's opinion.

[devnexen]

difference between them.

David is knowledgeable about UNIX stuff. Maybe he could decide this. cc @devnexen :)

It s nothing unix, just C :)

[devnexen]

@divinity76 the change is not wrong but I sense the "optimisation" part might not be remarkable across the board, depending on libc and platforms. Did you benchmark it a bit ?

To summarize: it is probably good to go, but a shift in the wording might be asked.

[LamentXU123]

I assume people familiar with unix is also familiar with this as well (^_^)

@divinity76 the change is not wrong but I sense the "optimisation" part might not be remarkable across the board, depending on libc and platforms. Did you benchmark it a bit ?

I am sure some compilers are smart enough to do this already in this case.

[divinity76]

@devnexen

Did you benchmark it a bit ?

No, In all likelihood It's only saving a couple of asm instructions to the tune of

; check if memcpy is safe
mov eax,src
add eax,len
cmp eax, dst
jl memcpy_is_safe

• this execute in nanoseconds on modern computers, and would be very difficult to benchmark.
• CPU Branch predictor makes it harder to benchmark
• The compiler may catch it at compile-time, in which case it's equally fast and impossible to benchmark.. ! my gcc 13.3.0 with -O2 did not catch it at compile-time, I checked:

- 5205fc: e8 2f 1f ce ff        call   202530 <memcpy@plt>
+ 5205fc: e8 cf 18 ce ff        call   201ed0 <memmove@plt>

To summarize: it is probably good to go, but a shift in the wording might be asked.

Suggestions? Would you prefer str_repeat nitpicking ?

[devnexen]

x86-64 (glibc) builds both from the same file, strong_alias (MEMMOVE_SYMBOL (__memmove, unaligned), MEMCPY_SYMBOL (__memcpy, unaligned)), so memcpy is the very same address, nothing saved. aarch64 shares the small-copy code up to 128 bytes, no overlap test at all, and beyond that memmove does sub / cbz / cmp / b.hs L(copy_long) into memcpy s own long path, so 4 instructions. musl just forwards, if ((uintptr_t)s-(uintptr_t)d-n <= -2*n) return memcpy(d, s, n);. And where memmove is a plainer loop instead, it is not a fixed branch anymore, it grows with l. And I m just talking about Linux here..

this is what I was trying to say with the "optimisation" part might not be remarkable across the board. Again not against at all even though I would say, let is give it time to someone to chime in just in case there is some context we re missing.