Pack the field array and the subs array into one block of contiguous memory.

Instead of giving every MiniTable two parallel arrays (fields and subs), we now have one block of memory that contains both lists.

For sub-message or enum fields, the `upb_MiniTableField` now contains a byte offset to where the `upb_MiniTable*` or `upb_MiniTableEnum*` can be found.

The new design offers several benefits:

1. The sub-table requires fewer pointer-chasing indirections to find.
2. We no longer need the `upb_MiniTable*` to find the sub-MiniTable -- the `upb_MiniTableField*` is enough now.  So APIs like `upb_MiniTable_GetSubMessageTable()` now require fewer parameters.
3. This unifies fields and extensions -- both can now find a sub-table using exactly the same code, with no need to distinguish between them.

We still benefit from having two lists.  If we tried to put the sub-table pointer directly into `upb_MiniTableField`, it would double the size of `upb_MiniTableField` from 12 to 24 bytes, due to alignment.  Using the current design, sub-message fields only take 20 bytes, while others take 12, so this design is an improvement even if 100% of fields are sub-message fields.

PiperOrigin-RevId: 824275694

Author: haberman

rust/upb/message.rs

@@ -158,14 +158,8 @@ impl<T: AssociatedMiniTable> MessagePtr<T> {
     ) -> Option<MessagePtr<ChildT>> {
         let f = unsafe { sys_mt::upb_MiniTable_GetFieldByIndex(T::mini_table(), index) };
 
-        let raw = unsafe {
-            sys_msg::upb_Message_GetOrCreateMutableMessage(
-                self.raw,
-                T::mini_table(),
-                f,
-                arena.raw(),
-            )
-        };
+        let raw =
+            unsafe { sys_msg::upb_Message_GetOrCreateMutableMessage(self.raw, f, arena.raw()) };
         raw.map(|raw| MessagePtr { raw, _phantom: PhantomData })
     }
 
@@ -243,7 +237,7 @@ impl<T: AssociatedMiniTable> MessagePtr<T> {
     ) -> Option<sys_map::RawMap> {
         unsafe {
             let f = sys_mt::upb_MiniTable_GetFieldByIndex(T::mini_table(), index);
-            let map_entry_mini_table = sys_mt::upb_MiniTable_SubMessage(T::mini_table(), f);
+            let map_entry_mini_table = sys_mt::upb_MiniTable_SubMessage(f);
             sys_msg::upb_Message_GetOrCreateMutableMap(
                 self.raw,
                 map_entry_mini_table,

rust/upb/sys/message/message.rs

@@ -127,7 +127,6 @@ unsafe extern "C" {
     /// - `f` must be a message-typed field associated with `m`
     pub fn upb_Message_GetOrCreateMutableMessage(
         m: RawMessage,
-        mini_table: RawMiniTable,
         f: RawMiniTableField,
         arena: RawArena,
     ) -> Option<RawMessage>;

rust/upb/sys/mini_table/mini_table.rs

@@ -75,7 +75,7 @@ unsafe extern "C" {
     /// # Safety
     /// - `m` and `f` must be valid to deref
     /// - `f` must be a mesage or map typed field associated with `m`
-    pub fn upb_MiniTable_SubMessage(m: RawMiniTable, f: RawMiniTableField) -> RawMiniTable;
+    pub fn upb_MiniTable_SubMessage(f: RawMiniTableField) -> RawMiniTable;
 
     /// Builds a mini table from the data encoded in the buffer [data, len]. If
     /// any errors occur, returns null and sets a status message if status is

upb/message/accessors.c

@@ -17,12 +17,11 @@
 // Must be last.
 #include "upb/port/def.inc"
 
-bool upb_Message_SetMapEntry(upb_Map* map, const upb_MiniTable* m,
-                             const upb_MiniTableField* f,
+bool upb_Message_SetMapEntry(upb_Map* map, const upb_MiniTableField* f,
                              upb_Message* map_entry_message, upb_Arena* arena) {
   UPB_ASSERT(!upb_Message_IsFrozen(map_entry_message));
   const upb_MiniTable* map_entry_mini_table =
-      upb_MiniTable_MapEntrySubMessage(m, f);
+      upb_MiniTable_MapEntrySubMessage(f);
   UPB_ASSERT(map_entry_mini_table);
   const upb_MiniTableField* map_entry_key_field =
       upb_MiniTable_MapKey(map_entry_mini_table);

upb/message/accessors.h

@@ -102,8 +102,7 @@ UPB_API_INLINE upb_Map* upb_Message_GetOrCreateMutableMap(
     const upb_MiniTableField* f, upb_Arena* arena);
 
 UPB_API_INLINE upb_Message* upb_Message_GetOrCreateMutableMessage(
-    upb_Message* msg, const upb_MiniTable* mini_table,
-    const upb_MiniTableField* f, upb_Arena* arena);
+    upb_Message* msg, const upb_MiniTableField* f, upb_Arena* arena);
 
 UPB_API_INLINE upb_StringView
 upb_Message_GetString(const upb_Message* msg, const upb_MiniTableField* field,
@@ -117,9 +116,9 @@ UPB_API_INLINE uint64_t upb_Message_GetUInt64(const upb_Message* msg,
                                               const upb_MiniTableField* f,
                                               uint64_t default_val);
 
-UPB_API_INLINE void upb_Message_SetClosedEnum(
-    upb_Message* msg, const upb_MiniTable* msg_mini_table,
-    const upb_MiniTableField* f, int32_t value);
+UPB_API_INLINE void upb_Message_SetClosedEnum(upb_Message* msg,
+                                              const upb_MiniTableField* f,
+                                              int32_t value);
 
 // BaseField Setters ///////////////////////////////////////////////////////////
 
@@ -302,14 +301,40 @@ UPB_API_INLINE const upb_MiniTableField* upb_Message_WhichOneof(
     const upb_MiniTableField* f);
 
 // Updates a map entry given an entry message.
-bool upb_Message_SetMapEntry(upb_Map* map, const upb_MiniTable* mini_table,
-                             const upb_MiniTableField* field,
+bool upb_Message_SetMapEntry(upb_Map* map, const upb_MiniTableField* field,
                              upb_Message* map_entry_message, upb_Arena* arena);
 
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
 
+#if defined(__cplusplus)
+// Temporary overloads for functions whose signature has recently changed.
+UPB_DEPRECATE_AND_INLINE()
+inline upb_Message* upb_Message_GetOrCreateMutableMessage(
+    upb_Message* msg, const upb_MiniTable* mini_table,
+    const upb_MiniTableField* f, upb_Arena* arena) {
+  return upb_Message_GetOrCreateMutableMessage(msg, f, arena);
+}
+
+UPB_DEPRECATE_AND_INLINE()
+inline void upb_Message_SetClosedEnum(upb_Message* msg,
+                                      const upb_MiniTable* msg_mini_table,
+                                      const upb_MiniTableField* f,
+                                      int32_t value) {
+  upb_Message_SetClosedEnum(msg, f, value);
+}
+
+UPB_DEPRECATE_AND_INLINE()
+inline bool upb_Message_SetMapEntry(upb_Map* map,
+                                    const upb_MiniTable* mini_table,
+                                    const upb_MiniTableField* field,
+                                    upb_Message* map_entry_message,
+                                    upb_Arena* arena) {
+  return upb_Message_SetMapEntry(map, field, map_entry_message, arena);
+}
+#endif
+
 #include "upb/port/undef.inc"
 
 #endif  // UPB_MESSAGE_ACCESSORS_H_

upb/message/accessors_test.cc

@@ -333,9 +333,7 @@ TEST(GeneratedCode, SubMessage) {
                          new_nested_message);
 
   upb_Message* mutable_message = upb_Message_GetOrCreateMutableMessage(
-      UPB_UPCAST(msg),
-      &protobuf_0test_0messages__proto2__TestAllTypesProto2_msg_init,
-      optional_message_field, arena);
+      UPB_UPCAST(msg), optional_message_field, arena);
   EXPECT_EQ(
       true,
       protobuf_test_messages_proto2_TestAllTypesProto2_optional_nested_message(
@@ -438,13 +436,9 @@ TEST(GeneratedCode, GetMutableMessage) {
   const upb_MiniTableField* optional_message_field =
       find_proto2_field(kFieldOptionalNestedMessage);
   upb_Message* msg1 = upb_Message_GetOrCreateMutableMessage(
-      UPB_UPCAST(msg),
-      &protobuf_0test_0messages__proto2__TestAllTypesProto2_msg_init,
-      optional_message_field, arena);
+      UPB_UPCAST(msg), optional_message_field, arena);
   upb_Message* msg2 = upb_Message_GetOrCreateMutableMessage(
-      UPB_UPCAST(msg),
-      &protobuf_0test_0messages__proto2__TestAllTypesProto2_msg_init,
-      optional_message_field, arena);
+      UPB_UPCAST(msg), optional_message_field, arena);
   // Verify that newly constructed sub message is stored in msg.
   EXPECT_EQ(msg1, msg2);
 

upb/message/compare.c

@@ -73,7 +73,7 @@ static bool _upb_Map_IsEqual(const upb_Map* map1, const upb_Map* map2,
   if (size1 != size2) return false;
 
   const upb_MiniTableField* f = upb_MiniTable_MapValue(m);
-  const upb_MiniTable* m2_value = upb_MiniTable_SubMessage(m, f);
+  const upb_MiniTable* m2_value = upb_MiniTable_SubMessage(f);
   const upb_CType ctype = upb_MiniTableField_CType(f);
 
   upb_MessageValue key, val1, val2;
@@ -109,7 +109,7 @@ static bool _upb_Message_BaseFieldsAreEqual(const upb_Message* msg1,
     if (!got1) return true;          // Loop termination condition.
     if (f1 != f2) return false;      // Must have identical fields set.
 
-    const upb_MiniTable* subm = upb_MiniTable_SubMessage(m, f1);
+    const upb_MiniTable* subm = upb_MiniTable_SubMessage(f1);
     const upb_CType ctype = upb_MiniTableField_CType(f1);
 
     bool eq;

upb/message/copy.c

@@ -97,7 +97,7 @@ upb_Map* upb_Map_DeepClone(const upb_Map* map, upb_CType key_type,
         upb_MiniTable_MapValue(map_entry_table);
     const upb_MiniTable* value_sub =
         upb_MiniTableField_CType(value_field) == kUpb_CType_Message
-            ? upb_MiniTable_GetSubMessageTable(map_entry_table, value_field)
+            ? upb_MiniTable_GetSubMessageTable(value_field)
             : NULL;
     upb_CType value_field_type = upb_MiniTableField_CType(value_field);
     if (!upb_Clone_MessageValue(&val, value_field_type, value_sub, arena)) {
@@ -116,8 +116,7 @@ static upb_Map* upb_Message_Map_DeepClone(const upb_Map* map,
                                           upb_Message* clone,
                                           upb_Arena* arena) {
   UPB_ASSERT(!upb_Message_IsFrozen(clone));
-  const upb_MiniTable* map_entry_table =
-      upb_MiniTable_MapEntrySubMessage(mini_table, f);
+  const upb_MiniTable* map_entry_table = upb_MiniTable_MapEntrySubMessage(f);
   UPB_ASSERT(map_entry_table);
 
   const upb_MiniTableField* key_field = upb_MiniTable_MapKey(map_entry_table);
@@ -161,12 +160,12 @@ static bool upb_Message_Array_DeepClone(const upb_Array* array,
                                         upb_Message* clone, upb_Arena* arena) {
   UPB_ASSERT(!upb_Message_IsFrozen(clone));
   UPB_PRIVATE(_upb_MiniTableField_CheckIsArray)(field);
-  upb_Array* cloned_array = upb_Array_DeepClone(
-      array, upb_MiniTableField_CType(field),
-      upb_MiniTableField_CType(field) == kUpb_CType_Message
-          ? upb_MiniTable_GetSubMessageTable(mini_table, field)
-          : NULL,
-      arena);
+  upb_Array* cloned_array =
+      upb_Array_DeepClone(array, upb_MiniTableField_CType(field),
+                          upb_MiniTableField_CType(field) == kUpb_CType_Message
+                              ? upb_MiniTable_GetSubMessageTable(field)
+                              : NULL,
+                          arena);
 
   // Clear out upb_Array* due to parent memcpy.
   upb_Message_SetBaseField(clone, field, &cloned_array);
@@ -198,7 +197,7 @@ upb_Message* _upb_Message_Copy(upb_Message* dst, const upb_Message* src,
           const upb_Message* sub_message = upb_Message_GetMessage(src, field);
           if (sub_message != NULL) {
             const upb_MiniTable* sub_message_table =
-                upb_MiniTable_GetSubMessageTable(mini_table, field);
+                upb_MiniTable_GetSubMessageTable(field);
             upb_Message* dst_sub_message =
                 upb_Message_DeepClone(sub_message, sub_message_table, arena);
             if (dst_sub_message == NULL) {

upb/message/internal/accessors.h

@@ -500,16 +500,14 @@ UPB_API_INLINE struct upb_Map* upb_Message_GetOrCreateMutableMap(
 }
 
 UPB_API_INLINE struct upb_Message* upb_Message_GetOrCreateMutableMessage(
-    struct upb_Message* msg, const upb_MiniTable* mini_table,
-    const upb_MiniTableField* f, upb_Arena* arena) {
+    struct upb_Message* msg, const upb_MiniTableField* f, upb_Arena* arena) {
   UPB_ASSERT(arena);
   UPB_ASSUME(upb_MiniTableField_CType(f) == kUpb_CType_Message);
   UPB_ASSUME(!upb_MiniTableField_IsExtension(f));
   struct upb_Message* sub_message =
       *UPB_PTR_AT(msg, f->UPB_ONLYBITS(offset), struct upb_Message*);
   if (!sub_message) {
-    const upb_MiniTable* sub_mini_table =
-        upb_MiniTable_SubMessage(mini_table, f);
+    const upb_MiniTable* sub_mini_table = upb_MiniTable_SubMessage(f);
     UPB_ASSERT(sub_mini_table);
     sub_message = _upb_Message_New(sub_mini_table, arena);
     *UPB_PTR_AT(msg, f->UPB_ONLYBITS(offset), struct upb_Message*) =
@@ -645,13 +643,12 @@ UPB_API_INLINE void upb_Message_SetBaseFieldUInt64(struct upb_Message* msg,
 }
 
 UPB_API_INLINE void upb_Message_SetClosedEnum(struct upb_Message* msg,
-                                              const upb_MiniTable* m,
                                               const upb_MiniTableField* f,
                                               int32_t value) {
   UPB_ASSERT(upb_MiniTableField_IsClosedEnum(f));
   UPB_ASSUME(UPB_PRIVATE(_upb_MiniTableField_GetRep)(f) == kUpb_FieldRep_4Byte);
-  UPB_ASSERT(
-      upb_MiniTableEnum_CheckValue(upb_MiniTable_GetSubEnumTable(m, f), value));
+  UPB_ASSERT(upb_MiniTableEnum_CheckValue(upb_MiniTable_GetSubEnumTable(f),
+                                          (uint32_t)value));
   upb_Message_SetBaseField(msg, f, &value);
 }
 

upb/message/message.c

@@ -251,7 +251,7 @@ void upb_Message_Freeze(upb_Message* msg, const upb_MiniTable* m) {
 
   for (size_t i = 0; i < field_count; i++) {
     const upb_MiniTableField* f = upb_MiniTable_GetFieldByIndex(m, i);
-    const upb_MiniTable* m2 = upb_MiniTable_SubMessage(m, f);
+    const upb_MiniTable* m2 = upb_MiniTable_SubMessage(f);
 
     switch (UPB_PRIVATE(_upb_MiniTableField_Mode)(f)) {
       case kUpb_FieldMode_Array: {
@@ -263,7 +263,7 @@ void upb_Message_Freeze(upb_Message* msg, const upb_MiniTable* m) {
         upb_Map* map = upb_Message_GetMutableMap(msg, f);
         if (map) {
           const upb_MiniTableField* f2 = upb_MiniTable_MapValue(m2);
-          const upb_MiniTable* m3 = upb_MiniTable_SubMessage(m2, f2);
+          const upb_MiniTable* m3 = upb_MiniTable_SubMessage(f2);
           upb_Map_Freeze(map, m3);
         }
         break;