Removed fasttable specialization for sub-message size.

haberman · copybara-github · commit 2476010b1801 · 2025-05-08T13:39:11.000-07:00
This will (temporarily) reduce the performance of sub-message parsing, but with the intention of reclaiming it later through runtime branches (like we do for strings).

I was leaning in this direction already, for several reasons:

1. The old code broke the encapsulation of `upb_Arena`. Going forward we'd like all arena manipulation to go through formally-defined interfaces in `arena.h`.

2. Specializing the table on sub-message size creates a tight coupling between the generated code for a message and its sub-message.  If the sub-message changed size and the containing message was not regenerated, we could get buffer overflows.  While we generally do not support ABI stability across generated code versions, this would be an especially painful issue to debug if it ever did occur.

3. Specializing field parsers on message size majorly increases the number of fast parser functions, increasing code size overhead.  This change should noticeably shrink the code size overhead of switching on fasttable parsing.

This CL shrinks the overhead of fasttable by ~36Ki:

```
$ bloaty --domain=vm blaze-bin/third_party/upb/upb/wire/decode_fast/libfield_parsers.a -- /google/src/cloud/haberman/clean/google3/blaze-bin/third_party/upb/upb/wire/decode_fast/libfield_parsers.a
     VM SIZE
 --------------
 -19.0%    -128    .rodata.cst16
 -27.9% -4.09Ki    .eh_frame
 -31.8% -31.3Ki    .text
 -31.3% -35.6Ki    TOTAL
```

PiperOrigin-RevId: 756439207
diff --git a/upb/wire/decode_fast/combinations.h b/upb/wire/decode_fast/combinations.h
@@ -18,9 +18,8 @@
 //
 // These can be generated in any combination, except that non-primitive types
 // cannot be packed.  So we have:
-//   {1bt,2bt} x {s,o,r,p} x {b,v32,v64,z32,z64,f32,f64,ce}  // Primitive types.
-//   {1bt,2bt} x {s,o,r}   x {s,b}                           // String types.
-//   {1bt,2bt} x {s,o,r}   x {m64,m128,m192,m256,mmax}       // Message types.
+//   {1bt,2bt} x {s,o,r,p} x {b,v32,v64,z32,z64,f32,f64,ce}  // Primitive
+//   {1bt,2bt} x {s,o,r}   x {s,b,m}                         // Non-primitive
 
 #define UPB_DECODEFAST_CARDINALITIES(F, ...) \
   F(__VA_ARGS__, Scalar)                     \
@@ -39,11 +38,7 @@
   F(__VA_ARGS__, ClosedEnum)         \
   F(__VA_ARGS__, String)             \
   F(__VA_ARGS__, Bytes)              \
-  F(__VA_ARGS__, Message64)          \
-  F(__VA_ARGS__, Message128)         \
-  F(__VA_ARGS__, Message192)         \
-  F(__VA_ARGS__, Message256)         \
-  F(__VA_ARGS__, MessageBig)
+  F(__VA_ARGS__, Message)
 
 #define UPB_DECODEFAST_TAGSIZES(F, ...) \
   F(__VA_ARGS__, Tag1Byte)              \
@@ -101,11 +96,7 @@ UPB_INLINE int upb_DecodeFast_ValueBytes(upb_DecodeFast_Type type) {
     case kUpb_DecodeFast_Varint64:
     case kUpb_DecodeFast_ZigZag64:
     case kUpb_DecodeFast_Fixed64:
-    case kUpb_DecodeFast_Message64:
-    case kUpb_DecodeFast_Message128:
-    case kUpb_DecodeFast_Message192:
-    case kUpb_DecodeFast_Message256:
-    case kUpb_DecodeFast_MessageBig:
+    case kUpb_DecodeFast_Message:
       return 8;
     case kUpb_DecodeFast_String:
     case kUpb_DecodeFast_Bytes:
@@ -125,21 +116,6 @@ UPB_INLINE bool upb_DecodeFast_IsZigZag(upb_DecodeFast_Type type) {
   }
 }
 
-UPB_INLINE int upb_DecodeFast_MessageCeilingBytes(upb_DecodeFast_Type type) {
-  switch (type) {
-    case kUpb_DecodeFast_Message64:
-      return 64;
-    case kUpb_DecodeFast_Message128:
-      return 128;
-    case kUpb_DecodeFast_Message192:
-      return 192;
-    case kUpb_DecodeFast_Message256:
-      return 256;
-    default:
-      return 0;
-  }
-}
-
 // The canonical ordering of functions, used by the arrays of function pointers
 // and function names.  This macro generates the full cross product of the
 // cardinality, type, and tag size enums.
diff --git a/upb/wire/decode_fast/field_message.c b/upb/wire/decode_fast/field_message.c
@@ -24,22 +24,11 @@
 #include "upb/port/def.inc"
 
 UPB_INLINE
-upb_Message* decode_newmsg_ceil(upb_Decoder* d, const upb_MiniTable* m,
-                                upb_DecodeFast_Type type) {
+upb_Message* decode_newmsg(upb_Decoder* d, const upb_MiniTable* m) {
   size_t size = m->UPB_PRIVATE(size);
-  char* msg_data;
-  size_t msg_ceil_bytes = upb_DecodeFast_MessageCeilingBytes(type);
-  if (UPB_LIKELY(type != kUpb_DecodeFast_MessageBig &&
-                 UPB_PRIVATE(_upb_ArenaHas)(&d->arena) >= msg_ceil_bytes)) {
-    UPB_ASSERT(size <= (size_t)msg_ceil_bytes);
-    msg_data = d->arena.UPB_PRIVATE(ptr);
-    // TODO: fix for Xsan
-    d->arena.UPB_PRIVATE(ptr) += size;
-    memset(msg_data, 0, msg_ceil_bytes);
-  } else {
-    msg_data = (char*)upb_Arena_Malloc(&d->arena, size);
-    memset(msg_data, 0, size);
-  }
+  // OPT: specialize for message size
+  char* msg_data = (char*)upb_Arena_Malloc(&d->arena, size);
+  memset(msg_data, 0, size);
   return (upb_Message*)msg_data;
 }
 
@@ -99,7 +88,7 @@ const char* fastdecode_tosubmsg(upb_EpsCopyInputStream* e, const char* ptr,
   submsg.msg = *dst;                                                      \
                                                                           \
   if (card == kUpb_DecodeFast_Repeated || UPB_LIKELY(!submsg.msg)) {      \
-    *dst = submsg.msg = decode_newmsg_ceil(d, subtablep, type);           \
+    *dst = submsg.msg = decode_newmsg(d, subtablep);                      \
   }                                                                       \
                                                                           \
   ptr += tagbytes;                                                        \
@@ -136,11 +125,7 @@ const char* fastdecode_tosubmsg(upb_EpsCopyInputStream* e, const char* ptr,
                       kUpb_DecodeFast_##tagsize);                              \
   }
 
-UPB_DECODEFAST_CARDINALITIES(UPB_DECODEFAST_TAGSIZES, F, Message64)
-UPB_DECODEFAST_CARDINALITIES(UPB_DECODEFAST_TAGSIZES, F, Message128)
-UPB_DECODEFAST_CARDINALITIES(UPB_DECODEFAST_TAGSIZES, F, Message192)
-UPB_DECODEFAST_CARDINALITIES(UPB_DECODEFAST_TAGSIZES, F, Message256)
-UPB_DECODEFAST_CARDINALITIES(UPB_DECODEFAST_TAGSIZES, F, MessageBig)
+UPB_DECODEFAST_CARDINALITIES(UPB_DECODEFAST_TAGSIZES, F, Message)
 
 #undef F
 #undef FASTDECODE_SUBMSG
diff --git a/upb/wire/decode_fast/select.c b/upb/wire/decode_fast/select.c
@@ -103,44 +103,31 @@ static bool upb_DecodeFast_GetFieldType(const upb_MiniTable* m,
 
   if (upb_MiniTableField_IsClosedEnum(field)) {
     *out_type = kUpb_DecodeFast_ClosedEnum;
-  } else if (type == kUpb_FieldType_Message) {
-    const upb_MiniTable* subm = upb_MiniTable_GetSubMessageTable(m, field);
-    size_t size = subm->UPB_PRIVATE(size);
-    if (size <= 64) {
-      *out_type = kUpb_DecodeFast_Message64;
-    } else if (size <= 128) {
-      *out_type = kUpb_DecodeFast_Message128;
-    } else if (size <= 192) {
-      *out_type = kUpb_DecodeFast_Message192;
-    } else if (size <= 256) {
-      *out_type = kUpb_DecodeFast_Message256;
-    } else {
-      *out_type = kUpb_DecodeFast_MessageBig;
-    }
-  } else {
-    static const int8_t types[] = {
-        [kUpb_FieldType_Bool] = kUpb_DecodeFast_Bool,
-        [kUpb_FieldType_Enum] = kUpb_DecodeFast_Varint32,
-        [kUpb_FieldType_Int32] = kUpb_DecodeFast_Varint32,
-        [kUpb_FieldType_UInt32] = kUpb_DecodeFast_Varint32,
-        [kUpb_FieldType_Int64] = kUpb_DecodeFast_Varint64,
-        [kUpb_FieldType_UInt64] = kUpb_DecodeFast_Varint64,
-        [kUpb_FieldType_Fixed32] = kUpb_DecodeFast_Fixed32,
-        [kUpb_FieldType_SFixed32] = kUpb_DecodeFast_Fixed32,
-        [kUpb_FieldType_Float] = kUpb_DecodeFast_Fixed32,
-        [kUpb_FieldType_Fixed64] = kUpb_DecodeFast_Fixed64,
-        [kUpb_FieldType_SFixed64] = kUpb_DecodeFast_Fixed64,
-        [kUpb_FieldType_Double] = kUpb_DecodeFast_Fixed64,
-        [kUpb_FieldType_SInt32] = kUpb_DecodeFast_ZigZag32,
-        [kUpb_FieldType_SInt64] = kUpb_DecodeFast_ZigZag64,
-        [kUpb_FieldType_String] = kUpb_DecodeFast_String,
-        [kUpb_FieldType_Bytes] = kUpb_DecodeFast_Bytes,
-    };
-
-    UPB_ASSERT(type < UPB_ARRAY_SIZE(types));
-    *out_type = types[type];
+    return true;
   }
 
+  static const int8_t types[] = {
+      [kUpb_FieldType_Bool] = kUpb_DecodeFast_Bool,
+      [kUpb_FieldType_Enum] = kUpb_DecodeFast_Varint32,
+      [kUpb_FieldType_Int32] = kUpb_DecodeFast_Varint32,
+      [kUpb_FieldType_UInt32] = kUpb_DecodeFast_Varint32,
+      [kUpb_FieldType_Int64] = kUpb_DecodeFast_Varint64,
+      [kUpb_FieldType_UInt64] = kUpb_DecodeFast_Varint64,
+      [kUpb_FieldType_Fixed32] = kUpb_DecodeFast_Fixed32,
+      [kUpb_FieldType_SFixed32] = kUpb_DecodeFast_Fixed32,
+      [kUpb_FieldType_Float] = kUpb_DecodeFast_Fixed32,
+      [kUpb_FieldType_Fixed64] = kUpb_DecodeFast_Fixed64,
+      [kUpb_FieldType_SFixed64] = kUpb_DecodeFast_Fixed64,
+      [kUpb_FieldType_Double] = kUpb_DecodeFast_Fixed64,
+      [kUpb_FieldType_SInt32] = kUpb_DecodeFast_ZigZag32,
+      [kUpb_FieldType_SInt64] = kUpb_DecodeFast_ZigZag64,
+      [kUpb_FieldType_String] = kUpb_DecodeFast_String,
+      [kUpb_FieldType_Bytes] = kUpb_DecodeFast_Bytes,
+      [kUpb_FieldType_Message] = kUpb_DecodeFast_Message,
+  };
+
+  UPB_ASSERT(type < UPB_ARRAY_SIZE(types));
+  *out_type = types[type];
   return true;
 }
 
