-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-133968: Add PyUnicodeWriter_WriteASCII() function #133973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Replace most PyUnicodeWriter_WriteUTF8() calls with PyUnicodeWriter_WriteASCII().
JSON benchmark: #133832 (comment)
Benchmark hidden because not significant (11): encode 100 floats, encode ascii string len=100, encode Unicode string len=100, encode 1000 integers, encode 1000 floats, encode 1000 "ascii" strings, encode ascii string len=1000, encode escaped string len=896, encode 10000 integers, encode 10000 floats, encode 10000 "ascii" strings Up to 1.20x faster to encode booleans is interesting knowing that these strings are very short: "true" (4 characters) and "false" (5 characters). |
@serhiy-storchaka: What do you think of this function? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits
@@ -1806,9 +1806,24 @@ object. | |||
See also :c:func:`PyUnicodeWriter_DecodeUTF8Stateful`. | |||
.. c:function:: int PyUnicodeWriter_WriteASCII(PyUnicodeWriter *writer, const char *str, Py_ssize_t size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the caller need to hold a thread state? That's not immediately clear to me with a string writing API.
const char *str, | ||
Py_ssize_t size) | ||
{ | ||
_PyUnicodeWriter *priv_writer = (_PyUnicodeWriter*)writer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to have sanity-check assertions like this:
_PyUnicodeWriter *priv_writer = (_PyUnicodeWriter*)writer; | |
assert(writer != NULL); | |
_PyUnicodeWriter *priv_writer = (_PyUnicodeWriter*)writer; |
If the caller needs a thread state, it would be good to call _Py_AssertHoldsTstate
as well.
Well, we had But We can add private |
Co-authored-by: Peter Bierma <[email protected]>
I don't think that it can become as fast or faster than a function which takes ASCII string as argument. If we know that the input string is ASCII, there is no need to scan the string for non-ASCII characters, and we can take the fast path. You're right that the UTF-8 decoder is already highly optimized. |
In short:
It's hard to beat |
Yes, although it was close, at least for moderately large strings. Could it be optimized even more? I don't know. But decision about |
I created capi-workgroup/decisions#65 issue. |
Benchmark:
On long strings (10,000 bytes), PyUnicodeWriter_WriteASCII() is up to 2x faster (1.36 us => 690 ns) than PyUnicodeWriter_WriteUTF8(). from _testcapi import PyUnicodeWriter
import pyperf
range_100 = range(100)
def bench_write_utf8(text, size):
writer = PyUnicodeWriter(0)
for _ in range_100:
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
writer.write_utf8(text, size)
def bench_write_ascii(text, size):
writer = PyUnicodeWriter(0)
for _ in range_100:
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
writer.write_ascii(text, size)
runner = pyperf.Runner()
sizes = (10, 100, 1_000, 10_000)
for size in sizes:
text = b'x' * size
runner.bench_func(f'write_utf8 size={size:,}', bench_write_utf8, text, size,
inner_loops=1_000)
for size in sizes:
text = b'x' * size
runner.bench_func(f'write_ascii size={size:,}', bench_write_ascii, text, size,
inner_loops=1_000) |
Do we know where the bottleneck is for long strings? |
Replace most PyUnicodeWriter_WriteUTF8() calls with PyUnicodeWriter_WriteASCII().
📚 Documentation preview 📚: https://cpython-previews--133973.org.readthedocs.build/