Skip to content

Removed upb_Decoder's stateful, bespoke logic for field lookup. #21707

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.

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

Merged
merged 1 commit into from
May 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 44 additions & 0 deletions upb/mini_table/internal/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,50 @@ UPB_API_INLINE bool upb_MiniTable_IsMessageSet(const struct upb_MiniTable* m) {
return m->UPB_PRIVATE(ext) == kUpb_ExtMode_IsMessageSet;
}

UPB_API_INLINE
const struct upb_MiniTableField* upb_MiniTable_FindFieldByNumber(
const struct upb_MiniTable* m, uint32_t number) {
const size_t i = ((size_t)number) - 1; // 0 wraps to SIZE_MAX

// Ideal case: index into dense fields
if (i < m->UPB_PRIVATE(dense_below)) {
UPB_ASSERT(m->UPB_ONLYBITS(fields)[i].UPB_ONLYBITS(number) == number);
return &m->UPB_ONLYBITS(fields)[i];
}

// Early exit if the field number is out of range.
int32_t hi = m->UPB_ONLYBITS(field_count) - 1;
if (hi < 0 || number > m->UPB_ONLYBITS(fields)[hi].UPB_ONLYBITS(number)) {
return NULL;
}

// Slow case: binary search
uint32_t lo = m->UPB_PRIVATE(dense_below);
const struct upb_MiniTableField* base = m->UPB_ONLYBITS(fields);
while (hi >= (int32_t)lo) {
uint32_t mid = (hi + lo) / 2;
uint32_t num = base[mid].UPB_ONLYBITS(number);
// These comparison operations allow, on ARM machines, to fuse all these
// branches into one comparison followed by two CSELs to set the lo/hi
// values, followed by a BNE to continue or terminate the loop. Since binary
// search branches are generally unpredictable (50/50 in each direction),
// this is a good deal. We use signed for the high, as this decrement may
// underflow if mid is 0.
int32_t hi_mid = mid - 1;
uint32_t lo_mid = mid + 1;
if (num == number) {
return &base[mid];
}
if (UPB_UNPREDICTABLE(num < number)) {
lo = lo_mid;
} else {
hi = hi_mid;
}
}

return NULL;
}

UPB_INLINE bool UPB_PRIVATE(_upb_MiniTable_IsEmpty)(
const struct upb_MiniTable* m) {
extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty);
Expand Down
38 changes: 0 additions & 38 deletions upb/mini_table/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,6 @@
// Must be last.
#include "upb/port/def.inc"

const upb_MiniTableField* upb_MiniTable_FindFieldByNumber(
const upb_MiniTable* m, uint32_t number) {
const size_t i = ((size_t)number) - 1; // 0 wraps to SIZE_MAX

// Ideal case: index into dense fields
if (i < m->UPB_PRIVATE(dense_below)) {
UPB_ASSERT(m->UPB_PRIVATE(fields)[i].UPB_PRIVATE(number) == number);
return &m->UPB_PRIVATE(fields)[i];
}

// Slow case: binary search
uint32_t lo = m->UPB_PRIVATE(dense_below);
int32_t hi = m->UPB_PRIVATE(field_count) - 1;
const upb_MiniTableField* base = m->UPB_PRIVATE(fields);
while (hi >= (int32_t)lo) {
uint32_t mid = (hi + lo) / 2;
uint32_t num = base[mid].UPB_ONLYBITS(number);
// These comparison operations allow, on ARM machines, to fuse all these
// branches into one comparison followed by two CSELs to set the lo/hi
// values, followed by a BNE to continue or terminate the loop. Since binary
// search branches are generally unpredictable (50/50 in each direction),
// this is a good deal. We use signed for the high, as this decrement may
// underflow if mid is 0.
int32_t hi_mid = mid - 1;
uint32_t lo_mid = mid + 1;
if (num == number) {
return &base[mid];
}
if (UPB_UNPREDICTABLE(num < number)) {
lo = lo_mid;
} else {
hi = hi_mid;
}
}

return NULL;
}

const upb_MiniTableField* upb_MiniTable_GetOneof(const upb_MiniTable* m,
const upb_MiniTableField* f) {
if (UPB_UNLIKELY(!upb_MiniTableField_IsInOneof(f))) {
Expand Down
2 changes: 1 addition & 1 deletion upb/mini_table/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ typedef struct upb_MiniTable upb_MiniTable;
extern "C" {
#endif

UPB_API const upb_MiniTableField* upb_MiniTable_FindFieldByNumber(
UPB_API_INLINE const upb_MiniTableField* upb_MiniTable_FindFieldByNumber(
const upb_MiniTable* m, uint32_t number);

UPB_API_INLINE const upb_MiniTableField* upb_MiniTable_GetFieldByIndex(
Expand Down
52 changes: 8 additions & 44 deletions upb/wire/decode.c
Original file line number Diff line number Diff line change
Expand Up @@ -965,55 +965,22 @@ UPB_NOINLINE const upb_MiniTableField* _upb_Decoder_FindExtensionField(
return &upb_Decoder_FieldNotFoundField;
}

static const upb_MiniTableField* _upb_Decoder_FindField(
upb_Decoder* d, const upb_MiniTable* t, uint32_t field_number,
uint32_t* last_field_index, int wire_type) {
static const upb_MiniTableField* _upb_Decoder_FindField(upb_Decoder* d,
const upb_MiniTable* t,
uint32_t field_number,
int wire_type) {
if (t == NULL) return &upb_Decoder_FieldNotFoundField;

uint32_t idx = ((uint32_t)field_number) - 1; // 0 wraps to UINT32_MAX
if (idx < t->UPB_PRIVATE(dense_below)) {
// Fastest case: index into dense fields, and don't update last_field_index.
return &t->UPB_PRIVATE(fields)[idx];
}

uint32_t field_count = t->UPB_PRIVATE(field_count);
if (t->UPB_PRIVATE(dense_below) < field_count) {
// Linear search non-dense fields. Resume scanning from last_field_index
// since fields are usually in order.
idx = *last_field_index;
uint32_t candidate;
do {
candidate = t->UPB_PRIVATE(fields)[idx].UPB_PRIVATE(number);
if (candidate == field_number) {
goto found;
}
} while (++idx < field_count);

if (UPB_LIKELY(field_number > candidate)) {
// The field number we encountered is larger than any of our known fields,
// so it's likely that subsequent ones will be too.
*last_field_index = idx - 1;
} else {
// Fields not in tag order - scan from beginning
for (idx = t->UPB_PRIVATE(dense_below); idx < *last_field_index; idx++) {
if (t->UPB_PRIVATE(fields)[idx].UPB_PRIVATE(number) == field_number) {
goto found;
}
}
}
}
const upb_MiniTableField* field =
upb_MiniTable_FindFieldByNumber(t, field_number);
if (field) return field;

if (d->extreg && t->UPB_PRIVATE(ext)) {
return _upb_Decoder_FindExtensionField(d, t, field_number,
t->UPB_PRIVATE(ext), wire_type);
}

return &upb_Decoder_FieldNotFoundField; // Unknown field.

found:
UPB_ASSERT(t->UPB_PRIVATE(fields)[idx].UPB_PRIVATE(number) == field_number);
*last_field_index = idx;
return &t->UPB_PRIVATE(fields)[idx];
}

static int _upb_Decoder_GetVarintOp(const upb_MiniTableField* field) {
Expand Down Expand Up @@ -1318,8 +1285,6 @@ UPB_NOINLINE
const char* _upb_Decoder_DecodeMessage(upb_Decoder* d, const char* ptr,
upb_Message* msg,
const upb_MiniTable* layout) {
uint32_t last_field_index = 0;

#if UPB_FASTTABLE
// The first time we want to skip fast dispatch, because we may have just been
// invoked by the fast parser to handle a case that it bailed on.
Expand Down Expand Up @@ -1358,8 +1323,7 @@ const char* _upb_Decoder_DecodeMessage(upb_Decoder* d, const char* ptr,
return ptr;
}

field = _upb_Decoder_FindField(d, layout, field_number, &last_field_index,
wire_type);
field = _upb_Decoder_FindField(d, layout, field_number, wire_type);
ptr = _upb_Decoder_DecodeWireValue(d, ptr, layout, field, wire_type, &val,
&op);

Expand Down
Loading