Skip to content

Commit d716b35

Browse files
committed
Avoid dynamic memcpy in varint encoding
This improves the write performance by about 12% in local tests, putting varints almost on par with fixed integers (at least from PHP land). In order to do this safely, we need to be certain that the encoding is going to succeed. The reason this was previously encoded on the stack first is to make sure that the function didn't corrupt people's data by terminating due to an error. This shouldn't be possible when we force TValue to be an unsigned type (avoiding the sort of arithmetic right-shifting bugs that plagued PM in the 0.16 days).
1 parent a7c6198 commit d716b35

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

Serializers.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,11 @@ static void writeInt24(unsigned char*& buffer, size_t& length, size_t& offset, T
267267

268268
template<typename TValue>
269269
static inline void writeUnsignedVarInt(unsigned char*& buffer, size_t& length, size_t& offset, TValue value) {
270+
static_assert(std::is_unsigned<TValue>::value, "TValue must be an unsigned type");
271+
270272
const auto TYPE_BITS = sizeof(TValue) * CHAR_BIT;
273+
274+
extendBuffer(buffer, length, offset, VarIntConstants::MAX_BYTES<TYPE_BITS>);
271275
char result[VarIntConstants::MAX_BYTES<TYPE_BITS>];
272276

273277
TValue remaining = value;
@@ -278,21 +282,17 @@ static inline void writeUnsignedVarInt(unsigned char*& buffer, size_t& length, s
278282
TValue nextRemaining = remaining >> VarIntConstants::BITS_PER_BYTE;
279283

280284
if (nextRemaining == 0) {
281-
result[i] = nextByte;
285+
buffer[i + offset] = nextByte;
282286

283287
auto usedBytes = i + 1;
284-
extendBuffer(buffer, length, offset, usedBytes);
285-
memcpy(&buffer[offset], &result[0], usedBytes);
286288
offset += usedBytes;
287289

288290
return;
289291
}
290292

291-
result[i] = nextByte | VarIntConstants::MSB_MASK;
293+
buffer[i + offset] = nextByte | VarIntConstants::MSB_MASK;
292294
remaining = nextRemaining;
293295
}
294-
295-
zend_value_error("Value too large to be encoded as a VarInt");
296296
}
297297

298298
template<typename TUnsignedType, typename TSignedType>

0 commit comments

Comments
 (0)