Add _PyBytesWriter API

[vstinner]

BPO	17742
Nosy	@pitrou, @vstinner, @bitdancer, @serhiy-storchaka
Files	bytes_writer.patch bytes_writer_cjkcodecs.patch bench_encoders.compare bench_encoders.py bytes_writer-2.patch bytes_writer-3.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-05-16.21:26:58.896>
created_at = <Date 2013-04-15.22:03:35.251>
labels = ['interpreter-core', 'type-feature', 'invalid']
title = 'Add _PyBytesWriter API'
updated_at = <Date 2013-05-16.21:26:58.895>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2013-05-16.21:26:58.895>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2013-05-16.21:26:58.896>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2013-04-15.22:03:35.251>
creator = 'vstinner'
dependencies = []
files = ['29877', '29878', '29914', '29928', '29931', '29961']
hgrepos = []
issue_num = 17742
keywords = ['patch']
message_count = 18.0
messages = ['187035', '187036', '187053', '187057', '187217', '187263', '187274', '187297', '187304', '187384', '187476', '187478', '187480', '187484', '187522', '187595', '187944', '189415']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'r.david.murray', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17742'
versions = ['Python 3.4']

[vstinner]

In Python 3.3, I added _PyUnicodeWriter API to factorize code handling a Unicode "buffer", just the code to allocate memory and resize the buffer if needed.

I propose to do the same with a new _PyBytesWriter API. The API is very similar to _PyUnicodeWriter:

• _PyBytesWriter_Init(writer)
• _PyBytesWriter_Prepare(writer, count)
• _PyBytesWriter_WriteStr(writer, bytes_obj)
• _PyBytesWriter_WriteChar(writer, ch)
• _PyBytesWriter_Finish(writer)
• _PyBytesWriter_Dealloc(writer)

The patch changes ASCII, Latin1, UTF-8 and charmap encoders to use _PyBytesWriter API. A second patch changes CJK encoders.

I did not run a benchmark yet. I wrote a patch to factorize the code, not the make the code faster.

Notes on performances:

• I peek the "small buffer allocated on the stack" idea from UTF-8 encoder, but the smaller buffer is always 500 bytes (instead of a size depending on the Unicode maximum character of the input Unicode string)

• _PyBytesWriter overallocates by 25% (when overallocation is enabled), whereas charmap encoders doubles the buffer:

  /* exponentially overallocate to minimize reallocations /
  if (requiredsize < 2outsize)
  requiredsize = 2*outsize;

• I didn't check if the allocation size is the same with the patch. min_size and overallocate attributes should be set correctly to not make the code slower.

• The code writing a single into a _PyUnicodeWriter buffer is inlined in unicodeobject.c. _PyBytesWriter API does not provide inlined function for the same purpose.

[vstinner]

See also bpo-17694.

[serhiy-storchaka]

Could you please provide the benchmarks results? I am afraid that it may hit a performance. Results on Windows are especially interesting.

[pitrou]

I did not run a benchmark yet. I wrote a patch to factorize the code,
not the make the code faster.

Your patches don't seem to reduce the line count, so I don't understand the point.

[vstinner]

Here is a benchmark. It looks like the overallocation is not configured correctly for charmap and UTF-8 encoders: it is always enabled for UTF-8 encoder (whereas it should only be enabled on the first call to an error handler), and never enabled for charmap encoder.

[serhiy-storchaka]

I run my own benchmarks and don't see any regression besides a random noise. Actually I see even speed up for some cases:

./python -m timeit -s "a = '\x80'*10000" "a.encode()"

Before patch: 29.8 usec per loop, after patch: 21.5 usec per loop.
This is just a compiler's caprice.

[serhiy-storchaka]

I have added some comments on Rietveld.

[vstinner]

Benchmark script, should be used with:
https://bitbucket.org/haypo/misc/src/tip/python/benchmark.py

[vstinner]

New version of the patch:

• address most (all?) Serhiy's remarks
• _PyBytesWriter_PrepareInternal() always use min_size, not only when overallocate is 1
• add more comments

Performances are almost the same than without the patch. It looks like they are a little bit better.

[serhiy-storchaka]

_PyBytesWriter and _PyUnicodeWriter have differen use cases. While _PyUnicodeWriter used primary in formatter where resulting size is rarely known and reallocation in decoders usually caused by widening result, _PyBytesWriter is used only in decoders where we usually can estimate a result size or it's upper bound. Resizing happened only in exceptional cases, when error handler called.

The patch contains a special case for writing only one bytes object. This is very unlikely case. It happened only when an encoded string contains only one illegal character. I think this case is not worth a complication and
obfuscation of the code. I think we should drop readonly attribute and a piece of the code (which looks buggy for me anyway).

[vstinner]

The patch contains a special case for writing only one bytes object.
This is very unlikely case.

The patch only modify a few functions to make them use the new _PyBytesWriter API. Other functions can use it.

A few examples:

• PyBytes_FromObject()
• binascii: binascii_rledecode_hqx()
• bz2, lzma and zlib modules
• marshal and pickle modules
• datetime.datetime.strftime()
• Python/compile.c: assemble_lnotab()
• more Unicode decoders

But I agree that the readonly "hack" can be removed from _PyBytesWriter API since the bytes type has no format method (no bytes%args nor bytes.format(args)).

[bitdancer]

It may eventually get one, though.

[vstinner]

It may eventually get one, though.

If a use case for the "read-only hack" comes, the hack can be added again later. It's better to start with something simple and extend it with new use cases.