Copilot commented on code in PR #3741:
URL: https://github.com/apache/thrift/pull/3741#discussion_r3839453969
##########
lib/rb/ext/compact_protocol.c:
##########
@@ -174,27 +174,29 @@ static int32_t message_seqid_from_varint32(uint32_t
seqid) {
}
static void write_varint32(VALUE transport, uint32_t n) {
- while (true) {
- if ((n & ~0x7FU) == 0U) {
- write_byte_direct(transport, n & 0x7FU);
- break;
- } else {
- write_byte_direct(transport, (n & 0x7FU) | 0x80U);
- n = n >> 7;
- }
+ unsigned char bytes[5];
+ long length = 0;
+
+ while ((n & ~0x7FU) != 0U) {
+ bytes[length++] = (n & 0x7FU) | 0x80U;
+ n >>= 7;
}
+ bytes[length++] = n;
+
+ WRITE(transport, (const char*)bytes, length);
Review Comment:
This changes an observable transport boundary, but the existing
compact-protocol specs only verify the final bytes and do not detect a
regression back to one `write` call per byte. Please add a native-path
transport spy covering multi-byte i32 and i64 values and asserting that each
varint is delivered in one write containing the complete encoding; this is the
behavior the optimization is intended to guarantee.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
