Description

vstinner

opened on Nov 19, 2013

Issue body actions

BPO	19653
Nosy	@pitrou, @vstinner, @ericsnowcurrently, @serhiy-storchaka
Files	repr_writer.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-11-20.15:26:25.002>
created_at = <Date 2013-11-19.13:00:58.075>
labels = ['invalid']
title = 'Generalize usage of _PyUnicodeWriter for repr(obj): add _PyObject_ReprWriter()'
updated_at = <Date 2013-11-20.15:26:25.001>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2013-11-20.15:26:25.001>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2013-11-20.15:26:25.002>
closer = 'vstinner'
components = []
creation = <Date 2013-11-19.13:00:58.075>
creator = 'vstinner'
dependencies = []
files = ['32701']
hgrepos = []
issue_num = 19653
keywords = ['patch']
message_count = 5.0
messages = ['203371', '203459', '203461', '203491', '203492']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'eric.snow', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue19653'
versions = ['Python 3.4']

Activity

1. vstinner commented on Nov 19, 2013

   

   vstinner

   on Nov 19, 2013

   MemberAuthor

   More actions

   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.
2. ericsnowcurrently commented on Nov 20, 2013

   

   ericsnowcurrently

   on Nov 20, 2013

   Member

   More actions

   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.
3. serhiy-storchaka commented on Nov 20, 2013

   

   serhiy-storchaka

   on Nov 20, 2013

   Member

   More actions

   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.
4. pitrou commented on Nov 20, 2013

   

   pitrou

   on Nov 20, 2013

   Member

   More actions

   Why would you want to speed up repr()? I'm -1 unless there's an use case.
5. vstinner commented on Nov 20, 2013

   

   vstinner

   on Nov 20, 2013

   MemberAuthor

   More actions

   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 :-)
6. vstinner

   closed this as completedon Nov 20, 2013
7. vstinner

   added

   invalid

   on Nov 20, 2013
8. ezio-melotti

   transferred this issue fromon Apr 10, 2022

Issue actions