Skip to content

Align message buffers and RowSet rows - #77

Open
fdcastel wants to merge 2 commits into
asfernandes:mainfrom
fdcastel:rowset-alignment
Open

fdcastel wants to merge 2 commits into
asfernandes:mainfrom
fdcastel:rowset-alignment

Conversation

@fdcastel

Copy link
Copy Markdown
Contributor

Addresses item 4 of #74 (feedback: "Probably should be getAlignedLength. But not only that, and not only there, buffer start should be aligned as well.").

Problem

  • RowSet placed rows getMessageLength() bytes apart, so every row after the first could start misaligned for its fields.
  • Every message buffer (Statement input/output, RequestMessage, RowSet) was a plain std::vector<std::byte>, so its start alignment depended entirely on the global allocator.

Changes

  • New MessageBuffer.h:
    • fbcpp::MessageBuffer is a std::vector<std::byte> with an aligned allocator (impl::AlignedAllocator, which uses operator new with std::align_val_t).
    • Its start is aligned to MESSAGE_BUFFER_ALIGNMENT (16 bytes), which covers any field of a Firebird message.
  • Statement: inMessage and outMessage are MessageBuffer, and so are the return types of getInputMessage() and getOutputMessage().
  • RequestMessage and impl::ParameterSetter: use MessageBuffer for the message.
  • RowSet:
    • The buffer is a MessageBuffer.
    • Rows are spaced by the new getRowStride(), which is the message length rounded up to MESSAGE_BUFFER_ALIGNMENT. getRawRow() still returns getMessageLength() bytes, and getRawBuffer() now holds getCount() * getRowStride() bytes.

Notes for review

  • Why not getAlignedLength(): IMessageMetadata::getAlignedLength() and getAlignment() only exist from interface version 4 (Firebird 4.0 clients), and Blobs cannot be created with a Firebird 3 client library #71 showed that Firebird 3 client libraries still matter. Rounding up to the fixed 16-byte alignment is always at least as strict, costs at most 15 bytes per row, and works with any client. If you'd rather use getAlignedLength() when the client supports it, I can change it.
  • Source compatibility: the return types of getInputMessage(), getOutputMessage() and RowSet::getRawBuffer() change from std::vector<std::byte> to MessageBuffer. Code that uses auto&, .data(), .size() or iterators compiles unchanged. Code that names std::vector<std::byte>& explicitly needs to change it to fbcpp::MessageBuffer&.

Tests

  • RowSetSuite/rowsAreAligned uses a result set whose message length isn't a multiple of 16. It checks that:
    • the statement's input and output buffers are aligned;
    • every RowSet row starts aligned;
    • the stride is a multiple of 16 and at least the message length;
    • the SMALLINT, BIGINT and DOUBLE values read back correctly.
  • The existing RowSet size checks now use getRowStride().

Full suite: 349 test cases pass (Windows, MSVC, Firebird 5.0.3 server).

Message buffers (Statement input/output, RequestMessage, RowSet) now use MessageBuffer, a std::vector<std::byte> whose start is aligned to MESSAGE_BUFFER_ALIGNMENT (16 bytes) through an aligned allocator, instead of relying on the global allocator's alignment.

RowSet rows are now spaced by getRowStride(), the message length rounded up to that alignment, so every row starts aligned (previously rows after the first could be misaligned). getAlignedLength() is not used because it requires a Firebird 4.0+ client.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant