Thanks for taking a look @vstinner! Feedback addressed.

Another question I have is whether we want to expose something like the following to handle the Greek letter sigma edge case:

int PyUnicode_ToLowerHandleSigma(Py_UCS4 *str, Py_UCS4 ch, Py_UCS4 *buffer, int size)

PyUnicode_ToLowerHandleSigma

Would you mind to elaborate? I'm not aware of this special case.

If this is indeed a hard constant which will never be changed in future Unicode standards

Even if it's a constant which will never (!) change, IMO it's better to request a size as an argument to make the caller responsible to check the buffer size. APIs which accept a pointer with no size are a bad pattern, like the deprecated gets() function.

Further feedback addressed.

Would you mind to elaborate? I'm not aware of this special case.

There's one special case, the Greek letter sigma, where the result of lower casing is context-specific. More specifically, Σ gets lower-cased to ς if it's at the end of the word or to σ otherwise. This is handled in lower_ucs4 right now.

There's one special case, the Greek letter sigma, where the result of lower casing is context-specific. More specifically, Σ gets lower-cased to ς if it's at the end of the word or to σ otherwise.

Oh, that's a tricky case. Proposed API takes a single character, so we don't know if Σ is at the end of a word or not. I don't think that it's worth it to handle this special case in proposed API.

Can you try to add tests to Modules/_testcapi/unicode.c and Lib/test/test_capi/test_unicode.py?

Done.

If we add too many parameters and runtime checks, this will make the API slower and more difficult to use.

All feedback addressed! Thanks for all the help and patience @vstinner!

Friendly ping.

I ran a benchmark on:

• with_size: Py_ssize_t PyUCS4_ToLower(Py_UCS4 ch, Py_UCS4 *res, Py_ssize_t size)
• without_size: Py_ssize_t PyUCS4_ToLower(Py_UCS4 ch, Py_UCS4 *res)

I used ASCII letters for my benchmark: letters A to I (10 letters).

Mean +- std dev: [with_size] 2.21 ns +- 0.02 ns -> [without_size] 1.91 ns +- 0.01 ns: 1.16x faster

IMO the difference is too low (0.3 ns, 1.16x faster) to justify removing the size parameter.

diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index d0c0b45c20c..c65e3db137b 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -2555,6 +2555,48 @@ toggle_reftrace_printer(PyObject *ob, PyObject *arg)
     Py_RETURN_NONE;
 }
 
+
+static PyObject *
+bench(PyObject *self, PyObject *args)
+{
+    Py_ssize_t loops;
+    if (!PyArg_ParseTuple(args, "n", &loops)) {
+        return NULL;
+    }
+    Py_UCS4 buffer[10];
+
+    PyTime_t start;
+    (void)PyTime_PerfCounterRaw(&start);
+    for (Py_ssize_t i=0; i < loops; i++) {
+        Py_ssize_t res;
+        res = PyUCS4_ToLower('A', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('B', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('C', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('D', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('E', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('F', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('G', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('H', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('I', buffer, 3);
+        if (res < 0) return NULL;
+        res = PyUCS4_ToLower('J', buffer, 3);
+        if (res < 0) return NULL;
+    }
+    PyTime_t end;
+    (void)PyTime_PerfCounterRaw(&end);
+
+    return PyFloat_FromDouble(PyTime_AsSecondsDouble(end - start));
+}
+
+
 static PyMethodDef TestMethods[] = {
     {"set_errno",               set_errno,                       METH_VARARGS},
     {"test_config",             test_config,                     METH_NOARGS},
@@ -2649,6 +2691,7 @@ static PyMethodDef TestMethods[] = {
     {"test_atexit", test_atexit, METH_NOARGS},
     {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL},
     {"toggle_reftrace_printer", toggle_reftrace_printer, METH_O},
+    {"bench", bench, METH_VARARGS},
     {NULL, NULL} /* sentinel */
 };
 

import pyperf, _testcapi
runner = pyperf.Runner()
runner.bench_time_func('bench', _testcapi.bench, inner_loops=10)

I ran a benchmark on:

• with_size: Py_ssize_t PyUCS4_ToLower(Py_UCS4 ch, Py_UCS4 *res, Py_ssize_t size)
• use_int: int PyUCS4_ToLower(Py_UCS4 ch, Py_UCS4 *res, int size)

I used ASCII letters for my benchmark: letters A to I (10 letters).

Mean +- std dev: [with_size] 2.21 ns +- 0.02 ns -> [use_int] 2.26 ns +- 0.02 ns: 1.02x slower

The difference is not significant (or a little bit slower).

It means that str.lower() will now be 16% slower. Not good.

@serhiy-storchaka:

It means that str.lower() will now be 16% slower. Not good.

I ran a microbenchmark on str.lower:

env/bin/python -m pyperf timeit -s 's="a"*1_000' 's.lower()'

Result:

Mean +- std dev: [ref] 431 ns +- 6 ns -> [change] 418 ns +- 6 ns: 1.03x faster

str.lower() becomes faster with this change, not slower. At least, it's not 16% slower.

UPDATE: Just to be sure, I ran again the benchmark using --rigorous option. I got similar results:

Mean +- std dev: [ref] 418 ns +- 9 ns -> [change] 411 ns +- 10 ns: 1.02x faster

Then you did not test with right data. If PyUCS4_ToLower() becomes 16% slower, str.lower() that calls it in tight loop should be 16% slower for some data. Perhaps in your tests other things dominated.

If PyUCS4_ToLower() becomes 16% slower, str.lower() that calls it in tight loop should be 16% slower for some data.

I'm not sure about this logic. We are talking about nanoseconds. Things get more complicated when the difference is smaller than 1 nanosecond.

This scales with the length of the string.

Well, I trust the benchmark numbers :) You can easily run the str.lower() benchmark if you don't trust numbers :-)