From 8c1c4a73a22be87f1caf607ccc06a8f906efae38 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 26 May 2026 17:11:02 -0700 Subject: [PATCH] Use direct offset computation for submessage lookups PiperOrigin-RevId: 921771119 --- .../debug_string_test.fasttable.txt | 8 +-- upb/wire/decode_fast/data.h | 14 ++--- upb/wire/decode_fast/field_message.c | 15 +++--- upb/wire/decode_fast/field_varint.c | 52 +++++++++++++------ upb/wire/decode_fast/select.c | 31 ++++++++--- 5 files changed, 77 insertions(+), 43 deletions(-) diff --git a/upb/mini_table/debug_string_test.fasttable.txt b/upb/mini_table/debug_string_test.fasttable.txt index a7222c3755a46..ab5af4db63aa1 100644 --- a/upb/mini_table/debug_string_test.fasttable.txt +++ b/upb/mini_table/debug_string_test.fasttable.txt @@ -133,9 +133,9 @@ MiniTable#0 { .field_number = 6 } FastTableEntry { - .field_data = 0000000000000000, + .field_data = 0018000002130038, .field_parser = [ptr] - .field_number = 0 + .field_number = 7 } FastTableEntry { .field_data = 001c000003000040, @@ -153,7 +153,7 @@ MiniTable#0 { .field_number = 10 } FastTableEntry { - .field_data = 00580000060a005a, + .field_data = 005800000614005a, .field_parser = [ptr] .field_number = 11 } @@ -238,7 +238,7 @@ MiniTable#3 { .field_number = 0 } FastTableEntry { - .field_data = 001000000000000a, + .field_data = 001000000002000a, .field_parser = [ptr] .field_number = 1 } diff --git a/upb/wire/decode_fast/data.h b/upb/wire/decode_fast/data.h index c4de7adbec3e3..067b874db4e39 100644 --- a/upb/wire/decode_fast/data.h +++ b/upb/wire/decode_fast/data.h @@ -18,28 +18,28 @@ // // 48 32 16 0 // |--------|--------|--------|--------|--------|--------|--------|--------| -// | offset (16) |case offset (16) |presence| index | exp. tag (16) | +// | offset (16) |case offset (16) |presence| subofs | exp. tag (16) | // |--------|--------|--------|--------|--------|--------|--------|--------| // // - `offset` is the offset of the field in the message struct. // - `case_offset` is the offset of the oneof selector for a oneof field // (or 0 if not a oneof field). // - `presence` is either hasbit index or field number for oneofs. -// - `field_index` is the index of the field in the mini table's fields -// array (or 0 if we don't need the field index to parse this field type). +// - `subofs` is the 8 byte shifted submessage offset from the fields array +// start into the subs array (or 0 if no sub). // - `expected_tag` is the expected value of the tag for this field. UPB_INLINE bool upb_DecodeFast_MakeData(uint64_t offset, uint64_t case_offset, - uint64_t presence, uint64_t field_index, + uint64_t presence, uint64_t subofs, uint64_t expected_tag, uint64_t* out_data) { if (offset > 0xffff || case_offset > 0xffff || presence > 0xff || - field_index > 0xff || expected_tag > 0xffff) { + subofs > 0xff || expected_tag > 0xffff) { return false; } *out_data = (offset << 48) | (case_offset << 32) | (presence << 24) | - (field_index << 16) | expected_tag; + (subofs << 16) | expected_tag; return true; } @@ -55,7 +55,7 @@ UPB_INLINE uint8_t upb_DecodeFastData_GetPresence(uint64_t data) { return data >> 24; } -UPB_INLINE uint8_t upb_DecodeFastData_GetFieldIndex(uint64_t data) { +UPB_INLINE uint8_t upb_DecodeFastData_GetSubofs(uint64_t data) { return data >> 16; } diff --git a/upb/wire/decode_fast/field_message.c b/upb/wire/decode_fast/field_message.c index 530d1930791d6..063d688388b6b 100644 --- a/upb/wire/decode_fast/field_message.c +++ b/upb/wire/decode_fast/field_message.c @@ -10,7 +10,7 @@ #include "upb/message/internal/message.h" #include "upb/message/message.h" -#include "upb/mini_table/field.h" +#include "upb/mini_table/internal/sub.h" #include "upb/mini_table/message.h" #include "upb/wire/decode.h" #include "upb/wire/decode_fast/cardinality.h" @@ -65,14 +65,11 @@ void upb_DecodeFast_Message(upb_Decoder* d, const char** ptr, upb_Message* msg, upb_DecodeFast_Type type, upb_DecodeFast_Cardinality card, upb_DecodeFast_TagSize tagsize, uint64_t data2) { - uint16_t submsg_idx = upb_DecodeFastData_GetFieldIndex(*data); - - // OPT: we could remove an indirection by precomputing the offset directly - // to the submessage table pointer, instead of doing an extra hop through the - // field. - const upb_MiniTableField* field = - upb_MiniTable_GetFieldByIndex(table, submsg_idx); - const upb_MiniTable* subtablep = upb_MiniTable_GetSubMessageTable(field); + uint32_t submsg_ofs = upb_DecodeFastData_GetSubofs(*data) * 8; + + const upb_MiniTableSubInternal* sub = UPB_PTR_AT( + table->UPB_ONLYBITS(fields), submsg_ofs, upb_MiniTableSubInternal); + const upb_MiniTable* subtablep = sub->UPB_PRIVATE(submsg); upb_DecodeFast_MessageContext ctx = {subtablep, card == kUpb_DecodeFast_Repeated}; diff --git a/upb/wire/decode_fast/field_varint.c b/upb/wire/decode_fast/field_varint.c index 004eb97965301..bc40c171e3081 100644 --- a/upb/wire/decode_fast/field_varint.c +++ b/upb/wire/decode_fast/field_varint.c @@ -9,16 +9,19 @@ #include #include +#include "upb/base/error_handler.h" #include "upb/base/internal/endian.h" +#include "upb/message/internal/message.h" #include "upb/message/message.h" #include "upb/mini_table/enum.h" -#include "upb/mini_table/field.h" +#include "upb/mini_table/internal/sub.h" #include "upb/mini_table/message.h" #include "upb/wire/decode.h" #include "upb/wire/decode_fast/cardinality.h" #include "upb/wire/decode_fast/combinations.h" #include "upb/wire/decode_fast/data.h" #include "upb/wire/decode_fast/dispatch.h" +#include "upb/wire/decode_fast/field_helpers.h" #include "upb/wire/decode_fast/field_parsers.h" #include "upb/wire/eps_copy_input_stream.h" #include "upb/wire/internal/decoder.h" @@ -35,12 +38,25 @@ typedef struct { } upb_DecodeFast_ClosedEnumContext; static UPB_PRESERVE_MOST void _upb_FastDecoder_AddEnumValueToUnknown( - upb_Decoder* d, upb_Message* msg, uint64_t data, uint64_t val, - const upb_MiniTable* table) { - uint16_t field_index = upb_DecodeFastData_GetFieldIndex(data); - const upb_MiniTableField* field = - upb_MiniTable_GetFieldByIndex(table, field_index); - _upb_Decoder_AddEnumValueToUnknown(d, msg, field, val); + upb_Decoder* d, upb_Message* msg, uint64_t data2, + upb_DecodeFast_TagSize tagsize, uint64_t val) { + uint16_t tag = upb_DecodeFastData2_GetOriginalTag(data2); + uint32_t field_num = (tagsize == kUpb_DecodeFast_Tag1Byte) + ? ((uint8_t)tag >> 3) + : _upb_DecodeFast_Tag2FieldNumber(tag); + // Original tag may have been packed, but we're storing single values as + // unpacked. + uint32_t varint_tag = (field_num << 3) | kUpb_WireType_Varint; + + char buf[2 * kUpb_Decoder_EncodeVarint32MaxSize]; + char* end = buf; + end = upb_Decoder_EncodeVarint32(varint_tag, end); + end = upb_Decoder_EncodeVarint32(val, end); + + if (!UPB_PRIVATE(_upb_Message_AddUnknown)(msg, buf, end - buf, &d->arena, + kUpb_AddUnknown_Copy)) { + upb_ErrorHandler_ThrowError(d->err, kUpb_DecodeStatus_OutOfMemory); + } } static bool upb_DecodeFast_SingleVarint(upb_Decoder* d, const char** ptr, @@ -84,7 +100,8 @@ typedef struct { uint64_t* data; uint64_t* hasbits; upb_DecodeFastNext* ret; - const upb_MiniTable* table; + uint64_t data2; + upb_DecodeFast_TagSize tagsize; const upb_MiniTableEnum* enum_table; } upb_DecodeFast_PackedVarintContext; @@ -136,8 +153,8 @@ static const char* upb_DecodeFast_PackedVarint(upb_EpsCopyInputStream* st, memcpy(arr.dst, &val32, 4); arr.dst = UPB_PTR_AT(arr.dst, 4, char); } else { - _upb_FastDecoder_AddEnumValueToUnknown(c->decoder, c->msg, *c->data, - val, c->table); + _upb_FastDecoder_AddEnumValueToUnknown(c->decoder, c->msg, c->data2, + c->tagsize, val); } } } else { @@ -165,10 +182,10 @@ void upb_DecodeFast_ClosedEnum(upb_Decoder* d, const char** ptr, upb_DecodeFastNext* ret, upb_DecodeFast_Cardinality card, upb_DecodeFast_TagSize tagsize, uint64_t data2) { - uint16_t field_index = upb_DecodeFastData_GetFieldIndex(*data); - const upb_MiniTableField* field = - upb_MiniTable_GetFieldByIndex(table, field_index); - const upb_MiniTableEnum* enum_table = upb_MiniTable_GetSubEnumTable(field); + uint32_t subenum_ofs = upb_DecodeFastData_GetSubofs(*data) * 8; + const upb_MiniTableSubInternal* sub = UPB_PTR_AT( + table->UPB_ONLYBITS(fields), subenum_ofs, upb_MiniTableSubInternal); + const upb_MiniTableEnum* enum_table = sub->UPB_PRIVATE(subenum); if (UPB_UNLIKELY(enum_table == NULL)) { UPB_DECODEFAST_EXIT(kUpb_DecodeFastNext_FallbackToMiniTable, ret); @@ -184,7 +201,8 @@ void upb_DecodeFast_ClosedEnum(upb_Decoder* d, const char** ptr, .data = data, .hasbits = hasbits, .ret = ret, - .table = table, + .data2 = data2, + .tagsize = tagsize, .enum_table = enum_table, }; upb_DecodeFast_Packed(d, ptr, kUpb_DecodeFast_ClosedEnum, card, tagsize, @@ -217,7 +235,7 @@ void upb_DecodeFast_ClosedEnum(upb_Decoder* d, const char** ptr, uint32_t val32 = val; memcpy(arr.dst, &val32, 4); } else { - _upb_FastDecoder_AddEnumValueToUnknown(d, msg, *data, val, table); + _upb_FastDecoder_AddEnumValueToUnknown(d, msg, data2, tagsize, val); arr.dst = UPB_PTR_AT(arr.dst, -4, char); } next_tag_matches = @@ -250,7 +268,7 @@ void upb_DecodeFast_ClosedEnum(upb_Decoder* d, const char** ptr, return; } } else { - _upb_FastDecoder_AddEnumValueToUnknown(d, msg, *data, val, table); + _upb_FastDecoder_AddEnumValueToUnknown(d, msg, data2, tagsize, val); } *ptr = p; } diff --git a/upb/wire/decode_fast/select.c b/upb/wire/decode_fast/select.c index c24230ba6b27d..f59fb13489e11 100644 --- a/upb/wire/decode_fast/select.c +++ b/upb/wire/decode_fast/select.c @@ -174,16 +174,35 @@ static bool upb_DecodeFast_GetFunctionData(const upb_MiniTable* m, upb_MiniTableField_IsInOneof(field) ? UPB_PRIVATE(_upb_MiniTableField_OneofOffset)(field) : 0; - uint64_t field_index = (upb_MiniTableField_IsSubMessage(field) || - upb_MiniTableField_IsClosedEnum(field)) - ? field - m->UPB_PRIVATE(fields) - : 0; + uint64_t subofs = 0; + if (upb_MiniTableField_IsSubMessage(field) || + upb_MiniTableField_IsClosedEnum(field)) { + uint64_t idx = field - m->UPB_PRIVATE(fields); + // Here we rely on the fact that sizeof(upb_MiniTableField) is the same on + // both 32 and 64 bit; if it wasn't, we could generate a bad offset if we + // compiled on a 32 bit machine targetting a 64 bit one. + UPB_STATIC_ASSERT(sizeof(upb_MiniTableField) % kUpb_SubmsgOffsetBytes == 0, + "upb_MiniTableField must be a multiple of the offset"); + uint64_t ofs_4byte = + idx * (sizeof(upb_MiniTableField) / kUpb_SubmsgOffsetBytes) + + field->UPB_PRIVATE(submsg_ofs); + // Fasttable is only supported on 64-bit platforms where pointers and the + // submessage entries (upb_MiniTableSubInternal) are 8 bytes, requiring + // 8-byte alignment. Since the base fields array is aligned to pointer size + // (at least 8 bytes), and each submessage entry must be 8-byte aligned, + // the total byte offset (4 * ofs_4byte) from the start of the fields array + // to the submessage entry is guaranteed to be a multiple of 8. + // Consequently, ofs_4byte is guaranteed to be even, and thus this offset + // can be scaled by 8 when loading. + UPB_ASSERT(ofs_4byte % 2 == 0); + subofs = ofs_4byte / 2; + } uint64_t presence; return upb_DecodeFast_GetPresence(field, &presence) && - upb_DecodeFast_MakeData(offset, case_offset, presence, field_index, - tag, out_data); + upb_DecodeFast_MakeData(offset, case_offset, presence, subofs, tag, + out_data); } static bool upb_DecodeFast_TryFillEntry(const upb_MiniTable* m,