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 < 2*outsize)
        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.

See also #17694.

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

> 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.

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.

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.

I have added some comments on Rietveld.

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

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.

_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).

> 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)).

It may eventually get one, though.

> 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.

I'm not completly satisfied of bytes_writer-2.patch. Most encoders work directly on a pointer (char*). If I want to keep the code using pointers (because it is efficient), I have to resynchronize the writer and the pointer before and after calling writer methods.

Here is a new patch implementing a different approach, closer to the current code. The patch version 3 has no "pos" attribute: the "position" is now a pointer (str). So encoders can just use this pointer instead of their own pointer.

I expect that replacing "*p++ = c;" with "*writer.str++ = c;" would not add an important overhead, especially because writer is a local variable, and str is the first attribute of the structure. I hope that the machine code will be exactly the same.

The last patch increases the size of the code substantially. I'm still wondering what the benefits are.

$ hg di --stat
 Include/bytesobject.h      |   90 ++++++++++++++
 Misc/NEWS                  |    3 +
 Objects/bytesobject.c      |  144 ++++++++++++++++++++++
 Objects/stringlib/codecs.h |  109 ++++------------
 Objects/unicodeobject.c    |  267 ++++++++++++++++-------------------------
 5 files changed, 368 insertions(+), 245 deletions(-)

"I expect that replacing "*p++ = c;" with "*writer.str++ = c;" would
not add an important overhead, especially because writer is a local
variable, and str is the first attribute of the structure. I hope that
the machine code will be exactly the same."

I ran some benchmarks: performances are better in some cases, but
worse in other cases. Performances are worse for simple loop like:

                while (collstart++<collend)
                    *writer.str++ = '?';

For the "writer.str++" instruction, the new pointer value is written
immediatly in the structure. The pointer value is also read again at
each iteration. So we have 1 load and 1 store per iteration. I read
the assembler code on x86_64 using GCC 4.2.1 of Mac OS 10.6.

I compared with the original code:

                while (collstart++<collend)
                    *str++ = '?';

GCC emits better code: str is stored in a register and the new value
of str is only written once, at the end of loop (instead of writing it
at each iteration). The pointer value is read before the loop. So we
have 0 load and 0 store (related to the pointer value) in the body of
the loop.

It may be an aliasing issue, but I didn't find how to say to GCC that
the new value of writer.str can be written only once at the end of the
loop. I tried to add __restrict__ keyword: the load (get the pointer
value) is moved out of the loop. But the store is still in the body of
the loop.

Advantages of the patch.

* finer control on how the buffer is allocated: only overallocate if the replacement string (while handling an encoding error) is longer than 1 byte/character. The "replace" error handler should never use overallocation for example. Overallocation (when misused, when it was not needed) has a cost at the end of the encoder, because the buffer must be resized (shrink)

* use a buffer allocated on the stack for short strings. I'm not really convinced of this optimization. The data is still copied when the result is converted to a bytes objects (PyBytes_FromStringAndSize). It may be interesting if the encoder has to handle one or more errors: no need to resize the buffer until we reach the size of the small buffer (ex: 512 bytes).

* handle correctly integer overflow: most encoders do not catch integer overflow errors and may fail to handle (very) long strings (ex: encoded string longer than PY_SSIZE_T_MAX).

I'm not convinced that the patch would permit to design faster code. According to the assembler, it is the opposite (when "*writer.str++" is used in a loop). I don't know if it's possible to design a more efficient _PyBytesWriter API (to help GCC to generate more efficient machine code), nor if the overhead is important in a "normal case" (bench_encoders.py tests border cases, text with many many errors).

_PyBytesWriter API makes the code slower and does not really reduce the number of lines, so I'm closing this issue as invalid.