This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Generalize usage of _PyUnicodeWriter for repr(obj): add _PyObject_ReprWriter()
Type: Stage:
Components: Versions: Python 3.4
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, pitrou, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2013-11-19 13:00 by vstinner, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
repr_writer.patch vstinner, 2013-11-19 13:00 review
Messages (5)
msg203371 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-11-19 13:00
The _PyUnicodeWriter API avoids creation of temporary Unicode strings and has very good performances to build Unicode strings with the PEP 393 (compact unicode string).

Attached patch adds a _PyObject_ReprWriter() function to avoid creation of tempory Unicode string while calling repr(obj) on containers like tuple, list or dict.

I did something similar for str%args and str.format(args).

To avoid the following code, we might add something to PyTypeObject, maybe a new tp_repr_writer field.

+    if (PyLong_CheckExact(v)) {
+        return _PyLong_FormatWriter(writer, v, 10, 0);
+    }
+    if (PyUnicode_CheckExact(v)) {
+        return _PyUnicode_ReprWriter(writer, v);
+    }
+    if (PyList_CheckExact(v)) {
+        return _PyList_ReprWriter(writer, v);
+    }
+    if (PyTuple_CheckExact(v)) {
+        return _PyTuple_ReprWriter(writer, v);
+    }
+    if (PyList_CheckExact(v)) {
+        return _PyList_ReprWriter(writer, v);
+    }
+    if (PyDict_CheckExact(v)) {
+        return _PyDict_ReprWriter(writer, v);
+    }

For example, repr(list(range(10))) ('[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]')  should only allocate one buffer of 37 bytes and then shink it to 30 bytes.

I guess that benchmarks are required to justify such changes.
msg203459 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-11-20 05:12
As far as I'm aware, the performance of __repr__() for any object is not much of a concern.  Repr is mostly for debugging and interactive use, so it's already fast/efficient enough for the target consumers: us. :)  Making __repr__() easier to write or maintain is worth it as long as benefit outweighs the cost of the churn.
msg203461 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-11-20 07:19
I supported patches for repr(list), repr(tuple) and repr(dict) because they made the code shorter and cleaner. But this patch is more questionable.
msg203491 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-20 15:17
Why would you want to speed up repr()? I'm -1 unless there's an use case.
msg203492 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-11-20 15:26
After writing the patch, I realized that it adds a lot of new functions and add more code. Then I asked myself if repr() is really an important function or not :-)

3 other developers ask the same question, so it's probably better to reject this huge patch :-)
History
Date User Action Args
2022-04-11 14:57:53adminsetgithub: 63852
2013-11-20 15:26:25vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg203492
2013-11-20 15:17:10pitrousetnosy: + pitrou
messages: + msg203491
2013-11-20 07:19:23serhiy.storchakasetmessages: + msg203461
2013-11-20 05:12:44eric.snowsetnosy: + eric.snow
messages: + msg203459
2013-11-19 13:00:58vstinnercreate