From 2dff287ac9988d53ddaaadf44a6e90f6abaf056a Mon Sep 17 00:00:00 2001 From: Anthony Printup <92564080+anthonyprintup@users.noreply.github.com> Date: Sat, 2 May 2026 22:28:59 +0200 Subject: [PATCH] perf: avoid redundant uint64 varint casts Emit direct varint_size calls for generated uint64 encoded-size expressions while preserving explicit casts for other scalar conversions. Call write_varint directly from the uint64 runtime writer and regenerate smoke outputs. Add focused Python codegen coverage plus a C++ smoke test for canonical uint64 varint bytes. --- smoke/generated/compat.protocyte.hpp | 2 +- smoke/generated/example.protocyte.hpp | 6 +- smoke/generated/protocyte/runtime/runtime.hpp | 2 +- smoke/src/host_smoke.cpp | 12 +++ src/protocyte/cpp.py | 6 ++ src/protocyte/runtime/runtime.hpp | 2 +- tests/test_plugin.py | 88 +++++++++++++++++++ 7 files changed, 112 insertions(+), 6 deletions(-) diff --git a/smoke/generated/compat.protocyte.hpp b/smoke/generated/compat.protocyte.hpp index e5ffda6..1062d7a 100644 --- a/smoke/generated/compat.protocyte.hpp +++ b/smoke/generated/compat.protocyte.hpp @@ -1809,7 +1809,7 @@ namespace protocyte_smoke::test::compat { if (f_uint64_ != 0u) { const auto st_size = ::protocyte::add_size( total, ::protocyte::tag_size(static_cast<::protocyte::u32>(FieldNumber::f_uint64)) + - ::protocyte::varint_size(static_cast<::protocyte::u64>(f_uint64_))); + ::protocyte::varint_size(f_uint64_)); if (!st_size) { return ::protocyte::unexpected(st_size.error()); } diff --git a/smoke/generated/example.protocyte.hpp b/smoke/generated/example.protocyte.hpp index 05e9b0c..5fcbd4e 100644 --- a/smoke/generated/example.protocyte.hpp +++ b/smoke/generated/example.protocyte.hpp @@ -4630,7 +4630,7 @@ namespace test::ultimate { { const auto st_size = ::protocyte::add_size( entry_payload, ::protocyte::tag_size(static_cast<::protocyte::u32>(EntryFieldNumber::key)) + - ::protocyte::varint_size(static_cast<::protocyte::u64>(entry.key))); + ::protocyte::varint_size(entry.key)); if (!st_size) { return st_size.status(); } @@ -4988,7 +4988,7 @@ namespace test::ultimate { if (f_uint64_ != 0u) { const auto st_size = ::protocyte::add_size( total, ::protocyte::tag_size(static_cast<::protocyte::u32>(FieldNumber::f_uint64)) + - ::protocyte::varint_size(static_cast<::protocyte::u64>(f_uint64_))); + ::protocyte::varint_size(f_uint64_)); if (!st_size) { return ::protocyte::unexpected(st_size.error()); } @@ -5319,7 +5319,7 @@ namespace test::ultimate { { const auto st_size = ::protocyte::add_size( entry_payload, ::protocyte::tag_size(static_cast<::protocyte::u32>(EntryFieldNumber::key)) + - ::protocyte::varint_size(static_cast<::protocyte::u64>(entry.key))); + ::protocyte::varint_size(entry.key)); if (!st_size) { return ::protocyte::unexpected(st_size.error()); } diff --git a/smoke/generated/protocyte/runtime/runtime.hpp b/smoke/generated/protocyte/runtime/runtime.hpp index 78c9af0..3c538e5 100644 --- a/smoke/generated/protocyte/runtime/runtime.hpp +++ b/smoke/generated/protocyte/runtime/runtime.hpp @@ -3765,7 +3765,7 @@ namespace protocyte { } template Status write_uint64(Writer &writer, const u64 value) noexcept { - return write_varint_scalar(writer, value); + return write_varint(writer, value); } template Status write_bool(Writer &writer, const bool value) noexcept { diff --git a/smoke/src/host_smoke.cpp b/smoke/src/host_smoke.cpp index eb70444..b83221d 100644 --- a/smoke/src/host_smoke.cpp +++ b/smoke/src/host_smoke.cpp @@ -2554,6 +2554,18 @@ TEST_CASE("tag_size matches protobuf group sizing", "[smoke][runtime]") { CHECK(protocyte::tag_size(large_field_number, protocyte::WireType::SGROUP) == single_tag_size * 2u); } +TEST_CASE("write_uint64 emits canonical varint bytes", "[smoke][runtime]") { + std::array encoded {}; + protocyte::SliceWriter writer(encoded.data(), encoded.size()); + + require_success(protocyte::write_uint64(writer, 0x8000000000000000ull)); + + const std::array expected {0x80u, 0x80u, 0x80u, 0x80u, 0x80u, + 0x80u, 0x80u, 0x80u, 0x80u, 0x01u}; + REQUIRE(writer.position() == expected.size()); + for (protocyte::usize index {}; index < expected.size(); ++index) { CHECK(encoded[index] == expected[index]); } +} + TEST_CASE("length-delimited sizes reject values that do not fit usize", "[smoke][runtime]") { if constexpr (sizeof(protocyte::usize) == sizeof(protocyte::u64)) { SUCCEED("usize can represent every u64 length on this target"); diff --git a/src/protocyte/cpp.py b/src/protocyte/cpp.py index 9c6e345..c5996e0 100644 --- a/src/protocyte/cpp.py +++ b/src/protocyte/cpp.py @@ -1916,10 +1916,16 @@ def _scalar_size(item: FieldModel, value: str) -> str: width = _fixed_scalar_width(item) if width is not None: return width + return _varint_size_expr(item, value) + + +def _varint_size_expr(item: FieldModel, value: str) -> str: if item.proto_type == FieldDescriptorProto.TYPE_SINT32: return f"::protocyte::varint_size(::protocyte::encode_zigzag32({value}))" if item.proto_type == FieldDescriptorProto.TYPE_SINT64: return f"::protocyte::varint_size(::protocyte::encode_zigzag64({value}))" + if item.proto_type == FieldDescriptorProto.TYPE_UINT64: + return f"::protocyte::varint_size({value})" return f"::protocyte::varint_size(static_cast<::protocyte::u64>({value}))" diff --git a/src/protocyte/runtime/runtime.hpp b/src/protocyte/runtime/runtime.hpp index 78c9af0..3c538e5 100644 --- a/src/protocyte/runtime/runtime.hpp +++ b/src/protocyte/runtime/runtime.hpp @@ -3765,7 +3765,7 @@ namespace protocyte { } template Status write_uint64(Writer &writer, const u64 value) noexcept { - return write_varint_scalar(writer, value); + return write_varint(writer, value); } template Status write_bool(Writer &writer, const bool value) noexcept { diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 9c21645..503320f 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1581,6 +1581,33 @@ def test_empty_message_comments_unused_writer_and_returns_zero_size() -> None: assert "return ::protocyte::usize {};" in header +def test_generated_encoded_size_omits_redundant_uint64_varint_casts() -> None: + request = plugin_pb2.CodeGeneratorRequest() + request.file_to_generate.append("uint64_casts.proto") + request.proto_file.append(_uint64_casts_file()) + + response = generate_response(request) + + assert not response.error + header = next(file.content for file in response.file if file.name == "uint64_casts.protocyte.hpp") + encoded_size_body = header.split( + "::protocyte::Result<::protocyte::usize> encoded_size() const noexcept {", maxsplit=1 + )[1].split("\n};", maxsplit=1)[0] + + assert "::protocyte::varint_size(version_)" in encoded_size_body + assert "::protocyte::varint_size(values_value)" in encoded_size_body + assert "::protocyte::varint_size(loose_values_value)" in encoded_size_body + assert "::protocyte::varint_size(choice.choice_value)" in encoded_size_body + assert "::protocyte::varint_size(entry.key)" in encoded_size_body + assert "::protocyte::varint_size(entry.value)" in encoded_size_body + assert "::protocyte::varint_size(static_cast<::protocyte::u64>(version_))" not in encoded_size_body + assert "::protocyte::varint_size(static_cast<::protocyte::u64>(values_value))" not in encoded_size_body + assert "::protocyte::varint_size(static_cast<::protocyte::u64>(loose_values_value))" not in encoded_size_body + assert "::protocyte::varint_size(static_cast<::protocyte::u64>(choice.choice_value))" not in encoded_size_body + assert "::protocyte::varint_size(static_cast<::protocyte::u64>(entry.key))" not in encoded_size_body + assert "::protocyte::varint_size(static_cast<::protocyte::u64>(entry.value))" not in encoded_size_body + + def test_generated_header_keeps_runtime_status_globally_qualified() -> None: request = plugin_pb2.CodeGeneratorRequest() request.file_to_generate.append("namespaced.proto") @@ -1688,6 +1715,67 @@ def _simple_file() -> descriptor_pb2.FileDescriptorProto: return file +def _uint64_casts_file() -> descriptor_pb2.FileDescriptorProto: + file = descriptor_pb2.FileDescriptorProto() + file.name = "uint64_casts.proto" + file.package = "demo" + file.syntax = "proto3" + + message = file.message_type.add() + message.name = "Uint64CastCases" + + field = message.field.add() + field.name = "version" + field.number = 1 + field.label = F.LABEL_OPTIONAL + field.type = F.TYPE_UINT64 + + field = message.field.add() + field.name = "values" + field.number = 2 + field.label = F.LABEL_REPEATED + field.type = F.TYPE_UINT64 + field.options.packed = True + + field = message.field.add() + field.name = "loose_values" + field.number = 5 + field.label = F.LABEL_REPEATED + field.type = F.TYPE_UINT64 + field.options.packed = False + + message.oneof_decl.add().name = "choice" + field = message.field.add() + field.name = "choice_value" + field.number = 3 + field.label = F.LABEL_OPTIONAL + field.type = F.TYPE_UINT64 + field.oneof_index = 0 + + entry = message.nested_type.add() + entry.name = "CountersEntry" + entry.options.map_entry = True + key = entry.field.add() + key.name = "key" + key.number = 1 + key.label = F.LABEL_OPTIONAL + key.type = F.TYPE_UINT64 + value = entry.field.add() + value.name = "value" + value.number = 2 + value.label = F.LABEL_OPTIONAL + value.type = F.TYPE_UINT64 + + field = message.field.add() + field.name = "counters" + field.number = 4 + field.label = F.LABEL_REPEATED + field.type = F.TYPE_MESSAGE + field.type_name = ".demo.Uint64CastCases.CountersEntry" + + return file + + def _constant_array_request() -> plugin_pb2.CodeGeneratorRequest: request = plugin_pb2.CodeGeneratorRequest() request.file_to_generate.append("arrays.proto")