Do you need a while loop or can you live with a for-loop here?

Let us keep the same style as before.

Is 32 optimal for any supported architecture? or is it possible to use 64-bit chunks for 64-bit architecture?

These are byte-chunks, not bitchunks. ARM64 and x86-64 have 16-byte (128-bit) vectors by default. Clang and GCC are able to use these properly. On other architectures the loop is simply unrolled.

MSVC does not unroll the loop however I see. That might have an impact on performance. I should test that.

These are byte-chunks, not bitchunks

Oh yes, sorry (well, my question remains the same actually).

On other architectures the loop is unrolled. Meaning it is going to be 32 compare and add instructions in a row.
This should be more optimal than looping, as the CPU can go on for a while until it hits a jump. Although the assembly does not look very elegant. Of course the performance impact of this can only be evaluated on these architectures, but I suspect it will be minimal.

Actually, I was wondering whether we could have a macro defining the correct constant to use depending on the architecture. That way, it could be more or less optimized per architecture. But if we do not already have that information, let's keep your 32.

That's a good idea. Let's see what happens when I get to the windows benchmarking. MSVC does not unroll the inner loop at all, so the performance is potentially going to be very poor. I was also thinking of enclosing the unrolled loop in compile guards and only allow it for architectures that are known to perform better this way. Anyway, I will get to that after some benchmarks in the coming days.

Alternatively, for MSVC, you could unroll the loop manually actually. While I may understand that it's maybe an overkill, it might be worthwhile I'd say.

If you put the count >= maxcount here, does the runtime increases a lot or not?

Yes. Then this PR makes no sense any more.

Because the current code tells the compiler that it:

• It can read the next 32 bytes as these are all valid memory
• Only counting of the character needs to be performed

If it needs to abort reading when the count == maxcount is made, it cannot use vectors to do the reading as 32 byte reads are not guaranteed.
So instead count is allowed to overshoot and a count >= maxcount check is placed outside the loop. This will mean that the function will read at most 31 bytes too much.

Maybe make that function private (I don't think it should be exposed except in this module).

It is private, as it is static. The STRINGLIB macro is to prevent name clobbering. This function will be generated for STRINGLIB_CHAR==Py_UCS1, Py_UCS2 and PyUCS4. I think keeping it this way is correct. But I may be wrong of course. How do you suggest making it private?

Oh just by adding an underscore before its name (I should have been clearer when I said "private", I meant it in the naming but I think we don't care about underscores in C files).

No, we don't use underscore prefix in Python for static functions. Moreover, the macro adds a prefix such as ucs1lib_.

No, we don't use underscore prefix in Python for static functions. Moreover, the macro adds a prefix such as ucs1lib_.

Maxcount is only used in the replace function, it is very unlikely that this condition will ever be triggered.

OK, but there's no reason to check for PY_SSIZE_T_MAX specifically when this works as well.

Yes you are correct. However, this function needs some refactoring, as this maxcount provision is only there for replace. Replace for single characters is special.cased elsewhere, so maxcount is actually always Pyssize_t_max I think. I want to revisit this at a later point.