pmmp / ext-encoding Goto Github PK

this takes away the hassle of dealing with intrinsics, since the library does it on our behalf, meaning basically free performance.

This is low priority right now due to other more effective ways of improving performance.

Currently around 2/3 of the overhead of write* comes from zend_string_init() and/or zend_string_alloc() due to emalloc() of tiny new strings.
These strings are almost immediately discarded in practice - after being appended to a buffer, they are no longer useful.
This means this is a giant waste of CPU time (though, it should be noted that this accounts for less than 1/3rd of the total time in benchmarks due to overhead added by PHP code).

This could be avoided by accepting a string by-reference to write bytes into instead of allocating new strings every time, although it would be better to have some dedicated type which we could reserve() bytes in (like a vector or similar).

This would also enable having large pooled reusable buffers for encoding, which would further improve performance.

The following things should be tested:

• Basic parity with appropriate pack() and unpack() codes with selected values
• Symmetry of encoding and decoding
• read*() must correctly handle non-reference types by throwing an error
• read*() must correctly update reference integer offset parameter if given
• read*() must correctly handle not being given an offset parameter
• read*() must read from the correct place when given an offset parameter
• writeUnsignedVarInt() must handle negative numbers properly (i.e. it must terminate)
• readUnsignedVarInt() must limit the number of bytes read and error appropriately
• read*() functions must error properly when not given enough bytes
• clone must work correctly on ByteBuffer
• serialize and unserialize must work correctly on ByteBuffer
• $buffer = new ByteBuffer($data) should behave the same as $buffer = new ByteBuffer(); $buffer->writeByteArray($data);

to be continued ...

If the caller wants to read a ByteBuffer for reading, the offset should be 0, so that the bytes can be read from the beginning.
If the caller wants to write a ByteBuffer, the offset should be placed at the end of the input data, as if writeByteArray() was used.

Possible solutions:

• Make the offset parameter mandatory
• Make a parameter that indicates whether the offset should be at the beginning or the end (slightly easier to use than a mandatory offset parameter)
• Make the constructor private and have ::reader() and ::writer() static factory functions
• Split ByteBuffer into a ByteBufferReader and ByteBufferWriter
• Separate read offset and write offset (effectively what BinaryStream was implicitly doing)

When appending to the end of a ByteBuffer, only as many bytes as needed are allocated, which will result in many allocations for large data if the initial buffer size was small.

The standard way to do this is to double the size of the buffer whenever it fills up. This is what, for example, std::vector does in C++.

A ByteBuffer which can read from a subsection of another ByteBuffer's memory would be useful for reducing allocations.

For example, Minecraft packet batches contain length-prefixed packet buffers. Each of these buffers currently has to be copied to a new BinaryStream via substr() in order to be decoded. This allocation could be avoided if BinaryStream were capable of directly reading the memory of the original stream.

This is a surprising omission, forcing the use of toString() which creates a copy of the buffer.

• BinaryStream::feof() - tells whether there are bytes left to read
• BinaryStream::getRemaining() - effectively substr(buffer, offset)