Skip to content

Removed indirection for sub-messages. #24165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Removed indirection for sub-messages. #24165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions hpb_generator/generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,15 @@ void WriteHeader(const google::protobuf::FileDescriptor* file, Context& ctx) {
}
ctx.Emit({{"class_name", ClassName(desc)},
{"minitable_name",
upb::generator::MiniTableMessagePtrVarName(desc->full_name())},
upb::generator::MiniTableMessageVarName(desc->full_name())},
{"outer_namespace", outer_namespace},
{"c_api_msg_type",
upb::generator::CApiMessageType(desc->full_name())}},
R"cc(
template <>
struct AssociatedUpbTypes<$outer_namespace$$class_name$> {
using CMessageType = $c_api_msg_type$;
static inline const upb_MiniTable* kMiniTable = $minitable_name$;
static inline const upb_MiniTable* kMiniTable = &$minitable_name$;
};
)cc");
}
Expand Down
6 changes: 1 addition & 5 deletions upb/mini_descriptor/decode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,15 +444,11 @@ static void upb_MtDecoder_AllocateSubs(upb_MtDecoder* d,
upb_SubCounts sub_counts) {
uint32_t total_count = sub_counts.submsg_count + sub_counts.subenum_count;
size_t subs_bytes = sizeof(*d->table.UPB_PRIVATE(subs)) * total_count;
size_t ptrs_bytes = sizeof(upb_MiniTable*) * sub_counts.submsg_count;
upb_MiniTableSubInternal* subs =
subs_bytes ? upb_MtDecoder_CheckedMalloc(d, subs_bytes) : NULL;
const upb_MiniTable** subs_ptrs =
ptrs_bytes ? upb_MtDecoder_CheckedMalloc(d, ptrs_bytes) : NULL;
uint32_t i = 0;
for (; i < sub_counts.submsg_count; i++) {
subs_ptrs[i] = NULL;
subs[i].UPB_PRIVATE(submsg) = &subs_ptrs[i];
subs[i].UPB_PRIVATE(submsg) = NULL;
}
if (sub_counts.subenum_count) {
upb_MiniTableField* f = d->fields;
Expand Down
2 changes: 1 addition & 1 deletion upb/mini_descriptor/link.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ bool upb_MiniTable_SetSubMessage(upb_MiniTable* table,
// TODO: Add this assert back once YouTube is updated to not call
// this function repeatedly.
// UPB_ASSERT(upb_MiniTable_GetSubMessageTable(table, field) == NULL);
memcpy((void*)table_subs[idx].UPB_PRIVATE(submsg), &sub, sizeof(void*));
table_subs[idx].UPB_PRIVATE(submsg) = sub;
return true;
}

Expand Down
8 changes: 4 additions & 4 deletions upb/mini_table/debug_string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ static void upb_MiniTablePrinter_PrintField(upb_MiniTablePrinter* p,
if (field->UPB_PRIVATE(submsg_index) != kUpb_NoSub) {
if (upb_MiniTableField_CType(field) == kUpb_CType_Message) {
int id = upb_MiniTablePrinter_GetIdForRef(
p, *mini_table->UPB_PRIVATE(subs)[field->UPB_PRIVATE(submsg_index)]
.UPB_PRIVATE(submsg));
p, mini_table->UPB_PRIVATE(subs)[field->UPB_PRIVATE(submsg_index)]
.UPB_PRIVATE(submsg));
upb_MiniTablePrinter_Printf(p, " .submsg = MiniTable#%d\n", id);
} else {
int id = upb_MiniTablePrinter_GetIdForRef(
Expand Down Expand Up @@ -292,8 +292,8 @@ static void upb_MiniTablePrinter_PrintMessage(upb_MiniTablePrinter* p,
if (field->UPB_PRIVATE(submsg_index) == kUpb_NoSub) continue;
if (upb_MiniTableField_CType(field) == kUpb_CType_Message) {
upb_MiniTablePrinter_PrintMessage(
p, *mini_table->UPB_PRIVATE(subs)[field->UPB_PRIVATE(submsg_index)]
.UPB_PRIVATE(submsg));
p, mini_table->UPB_PRIVATE(subs)[field->UPB_PRIVATE(submsg_index)]
.UPB_PRIVATE(submsg));
} else {
upb_MiniTablePrinter_PrintEnum(
p, mini_table->UPB_PRIVATE(subs)[field->UPB_PRIVATE(submsg_index)]
Expand Down
2 changes: 1 addition & 1 deletion upb/mini_table/internal/message.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ UPB_API_INLINE const struct upb_MiniTableField* upb_MiniTable_GetFieldByIndex(
UPB_INLINE const struct upb_MiniTable* UPB_PRIVATE(
_upb_MiniTable_GetSubTableByIndex)(const struct upb_MiniTable* m,
uint32_t i) {
return *m->UPB_PRIVATE(subs)[i].UPB_PRIVATE(submsg);
return m->UPB_PRIVATE(subs)[i].UPB_PRIVATE(submsg);
}

UPB_API_INLINE const struct upb_MiniTable* upb_MiniTable_SubMessage(
Expand Down
7 changes: 2 additions & 5 deletions upb/mini_table/internal/sub.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,13 @@
// Must be last.
#include "upb/port/def.inc"

typedef union {
const struct upb_MiniTable* const* UPB_PRIVATE(submsg);
const struct upb_MiniTableEnum* UPB_PRIVATE(subenum);
} upb_MiniTableSubInternal;

union upb_MiniTableSub {
const struct upb_MiniTable* UPB_PRIVATE(submsg);
const struct upb_MiniTableEnum* UPB_PRIVATE(subenum);
};

typedef union upb_MiniTableSub upb_MiniTableSubInternal;

#ifdef __cplusplus
extern "C" {
#endif
Expand Down
2 changes: 1 addition & 1 deletion upb/mini_table/message_benchmark.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ static void BM_FindFieldByNumber(benchmark::State& state) {
max = 552;
}
const upb_MiniTable* ptr =
third_0party_0upb_0upb_0mini_0table__TestManyFields_msg_init_ptr;
&third_0party_0upb_0upb_0mini_0table__TestManyFields_msg_init;
absl::BitGen bitgen;
uint32_t search[1024];
for (auto& s : search) {
Expand Down
4 changes: 2 additions & 2 deletions upb/text/encode_debug_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ std::string GetDebugString(const upb_Message* input,
}

TEST(TextNoReflection, ExtensionsString) {
const upb_MiniTable* mt_main = upb_0test__ModelWithExtensions_msg_init_ptr;
const upb_MiniTable* mt_main = &upb_0test__ModelWithExtensions_msg_init;
upb_Arena* arena = upb_Arena_New();

upb_test_ModelExtension1* extension1 = upb_test_ModelExtension1_new(arena);
Expand All @@ -53,7 +53,7 @@ TEST(TextNoReflection, ExtensionsString) {
}

TEST(TextNoReflection, ExtensionsInt) {
const upb_MiniTable* mt_main = upb_0test__ModelWithExtensions_msg_init_ptr;
const upb_MiniTable* mt_main = &upb_0test__ModelWithExtensions_msg_init;
upb_Arena* arena = upb_Arena_New();

upb_test_ModelExtension2* extension2 = upb_test_ModelExtension2_new(arena);
Expand Down
14 changes: 3 additions & 11 deletions upb/wire/decode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ typedef union {
// from an array of MiniTableSubs.
static const upb_MiniTable* _upb_MiniTableSubs_MessageByField(
const upb_MiniTableSubInternal* subs, const upb_MiniTableField* field) {
return *subs[field->UPB_PRIVATE(submsg_index)].UPB_PRIVATE(submsg);
return subs[field->UPB_PRIVATE(submsg_index)].UPB_PRIVATE(submsg);
}

// Returns the MiniTableEnum corresponding to a given MiniTableField
Expand Down Expand Up @@ -925,7 +925,7 @@ void _upb_Decoder_CheckUnlinked(upb_Decoder* d, const upb_MiniTable* mt,
do {
UPB_ASSERT(upb_MiniTableField_CType(oneof) == kUpb_CType_Message);
const upb_MiniTable* oneof_sub =
*mt->UPB_PRIVATE(subs)[oneof->UPB_PRIVATE(submsg_index)].UPB_PRIVATE(
mt->UPB_PRIVATE(subs)[oneof->UPB_PRIVATE(submsg_index)].UPB_PRIVATE(
submsg);
UPB_ASSERT(!oneof_sub);
} while (upb_MiniTable_NextOneofField(mt, &oneof));
Expand Down Expand Up @@ -1078,7 +1078,6 @@ const char* _upb_Decoder_DecodeKnownField(upb_Decoder* d, const char* ptr,
int op, wireval* val) {
const upb_MiniTableSubInternal* subs = layout->UPB_PRIVATE(subs);
uint8_t mode = field->UPB_PRIVATE(mode);
upb_MiniTableSubInternal ext_sub;

if (UPB_UNLIKELY(mode & kUpb_LabelFlags_IsExtension)) {
const upb_MiniTableExtension* ext_layout =
Expand All @@ -1090,14 +1089,7 @@ const char* _upb_Decoder_DecodeKnownField(upb_Decoder* d, const char* ptr,
}
d->original_msg = msg;
msg = &ext->data.UPB_PRIVATE(ext_msg_val);
if (upb_MiniTableField_IsSubMessage(&ext->ext->UPB_PRIVATE(field))) {
ext_sub.UPB_PRIVATE(submsg) =
&ext->ext->UPB_PRIVATE(sub).UPB_PRIVATE(submsg);
} else {
ext_sub.UPB_PRIVATE(subenum) =
ext->ext->UPB_PRIVATE(sub).UPB_PRIVATE(subenum);
}
subs = &ext_sub;
subs = &ext->ext->UPB_PRIVATE(sub);
}

switch (mode & kUpb_FieldMode_Mask) {
Expand Down
4 changes: 2 additions & 2 deletions upb/wire/encode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
// from an array of MiniTableSubs.
static const upb_MiniTable* _upb_Encoder_GetSubMiniTable(
const upb_MiniTableSubInternal* subs, const upb_MiniTableField* field) {
return *subs[field->UPB_PRIVATE(submsg_index)].UPB_PRIVATE(submsg);
return subs[field->UPB_PRIVATE(submsg_index)].UPB_PRIVATE(submsg);
}

static uint32_t encode_zz32(int32_t n) {
Expand Down Expand Up @@ -698,7 +698,7 @@ static char* encode_ext(char* ptr, upb_encstate* e,
} else {
upb_MiniTableSubInternal sub;
if (upb_MiniTableField_IsSubMessage(&ext->UPB_PRIVATE(field))) {
sub.UPB_PRIVATE(submsg) = &ext->UPB_PRIVATE(sub).UPB_PRIVATE(submsg);
sub.UPB_PRIVATE(submsg) = ext->UPB_PRIVATE(sub).UPB_PRIVATE(submsg);
} else {
sub.UPB_PRIVATE(subenum) = ext->UPB_PRIVATE(sub).UPB_PRIVATE(subenum);
}
Expand Down
39 changes: 21 additions & 18 deletions upb_generator/minitable/generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ std::string MessageVarName(upb::MessageDefPtr message) {
return MiniTableMessageVarName(message.full_name());
}

std::string MessagePtrVarName(upb::MessageDefPtr message) {
return MiniTableMessagePtrVarName(message.full_name());
}

std::string EnumVarName(upb::EnumDefPtr e) {
return MiniTableEnumVarName(e.full_name());
}
Expand Down Expand Up @@ -87,8 +83,7 @@ void WriteMessageField(upb::FieldDefPtr field,
std::string GetSub(upb::FieldDefPtr field, bool is_extension) {
if (auto message_def = field.message_type()) {
return absl::Substitute("{.UPB_PRIVATE(submsg) = &$0}",
is_extension ? MessageVarName(message_def)
: MessagePtrVarName(message_def));
MessageVarName(message_def));
}

if (auto enum_def = field.enum_subdef()) {
Expand Down Expand Up @@ -128,10 +123,19 @@ void WriteMessage(upb::MessageDefPtr message, const DefPoolPair& pools,
if (options.one_output_per_message && field.IsSubMessage() &&
IsCrossFile(field) && !upb_MiniTableField_IsMap(f)) {
if (seen.insert(pools.GetMiniTable64(field.message_type())).second) {
output(
"__attribute__((weak)) const upb_MiniTable* const $0 ="
" &UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken);\n",
MessagePtrVarName(field.message_type()));
output(R"cc(
__attribute__((weak)) const upb_MiniTable $0 = {
.UPB_PRIVATE(subs) = NULL,
.UPB_PRIVATE(fields) = NULL,
.UPB_PRIVATE(size) = sizeof(struct upb_Message),
.UPB_PRIVATE(field_count) = 0,
.UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable,
.UPB_PRIVATE(dense_below) = 0,
.UPB_PRIVATE(table_mask) = -1,
.UPB_PRIVATE(required_count) = 0,
};
)cc",
MessageVarName(field.message_type()));
}
}
}
Expand Down Expand Up @@ -205,8 +209,9 @@ void WriteMessage(upb::MessageDefPtr message, const DefPoolPair& pools,
output(" })\n");
}
output("};\n\n");
output("const upb_MiniTable* const $0 = &$1;\n", MessagePtrVarName(message),
MessageVarName(message));
// output("const upb_MiniTable* const $0 = &$1;\n",
// MessagePtrVarName(message),
// MessageVarName(message));
}

void WriteEnum(upb::EnumDefPtr e, Output& output) {
Expand Down Expand Up @@ -281,8 +286,6 @@ void WriteMiniTableHeader(const DefPoolPair& pools, upb::FileDefPtr file,

for (auto message : this_file_messages) {
output("extern const upb_MiniTable $0;\n", MessageVarName(message));
output("extern const upb_MiniTable* const $0;\n",
MessagePtrVarName(message));
}
for (auto ext : this_file_exts) {
output("extern const upb_MiniTableExtension $0;\n", ExtensionVarName(ext));
Expand Down Expand Up @@ -351,10 +354,10 @@ void WriteMiniTableSource(const DefPoolPair& pools, upb::FileDefPtr file,
std::vector<upb::EnumDefPtr> enums = SortedEnums(file, kClosedEnums);

if (options.one_output_per_message) {
for (auto message : messages) {
output("extern const upb_MiniTable* const $0;\n",
MessagePtrVarName(message));
}
// for (auto message : messages) {
// output("extern const upb_MiniTable* const $0;\n",
// MessagePtrVarName(message));
// }
for (const auto e : enums) {
output("extern const upb_MiniTableEnum $0;\n", EnumVarName(e));
}
Expand Down
4 changes: 0 additions & 4 deletions upb_generator/minitable/names.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ std::string MiniTableMessageVarName(absl::string_view full_name) {
return MangleName(full_name) + "_msg_init";
}

std::string MiniTableMessagePtrVarName(absl::string_view full_name) {
return MiniTableMessageVarName(full_name) + "_ptr";
}

std::string MiniTableEnumVarName(absl::string_view full_name) {
return MangleName(full_name) + "_enum_init";
}
Expand Down
6 changes: 0 additions & 6 deletions upb_generator/minitable/names.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ UPBC_API std::string MiniTableEnumVarName(absl::string_view full_name);
UPBC_API std::string MiniTableExtensionVarName(absl::string_view full_name);
UPBC_API std::string MiniTableFileVarName(absl::string_view proto_filename);

// This is used for weak linking and tree shaking. Other translation units may
// define weak versions of this symbol that point to a dummy message, to
// gracefully degrade the behavior of the generated code when the message is not
// linked into the current binary.
UPBC_API std::string MiniTableMessagePtrVarName(absl::string_view full_name);

} // namespace generator
} // namespace upb

Expand Down
Loading