From b64c6e193b52e079c04b77530cd6da84b2beddd2 Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Tue, 5 Dec 2023 08:57:27 -0800 Subject: [PATCH] upb: implement upb_Message_ShallowClone()/Copy() PiperOrigin-RevId: 588088145 --- php/ext/google/protobuf/message.c | 7 ++---- ruby/ext/google/protobuf_c/message.c | 7 ++---- upb/message/copy.c | 26 ++++++++++++++++----- upb/message/copy.h | 15 ++++++++---- upb/message/internal/message.h | 2 +- upb/mini_descriptor/decode.c | 16 +++++++------ upb/mini_descriptor/internal/encode_test.cc | 10 ++++---- upb/mini_table/internal/message.c | 2 +- upb/mini_table/internal/message.h | 2 +- upb/wire/decode_fast.c | 4 ++-- upb_generator/protoc-gen-upb.cc | 4 ++-- upb_generator/protoc-gen-upb_minitable.cc | 4 ++-- 12 files changed, 58 insertions(+), 41 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index af7cfc6cf1..979f226fc9 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -400,11 +400,8 @@ static zval* Message_get_property_ptr_ptr(zend_object* object, static zend_object* Message_clone_obj(zend_object* object) { Message* intern = (Message*)object; const upb_MiniTable* t = upb_MessageDef_MiniTable(intern->desc->msgdef); - upb_Message* clone = upb_Message_New(t, Arena_Get(&intern->arena)); - - // TODO: copy unknown fields? - // TODO: use official upb msg copy function - memcpy(clone, intern->msg, t->size); + upb_Message* clone = + upb_Message_ShallowClone(intern->msg, t, Arena_Get(&intern->arena)); zval ret; Message_GetPhpWrapper(&ret, intern->desc, clone, &intern->arena); return Z_OBJ_P(&ret); diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index d64d411f89..dfb586e8e9 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -660,11 +660,8 @@ static VALUE Message_dup(VALUE _self) { Message* self = ruby_to_Message(_self); VALUE new_msg = rb_class_new_instance(0, NULL, CLASS_OF(_self)); Message* new_msg_self = ruby_to_Message(new_msg); - size_t size = upb_MessageDef_MiniTable(self->msgdef)->size; - - // TODO - // TODO - memcpy((upb_Message*)new_msg_self->msg, self->msg, size); + const upb_MiniTable* m = upb_MessageDef_MiniTable(self->msgdef); + upb_Message_ShallowCopy((upb_Message*)new_msg_self->msg, self->msg, m); Arena_fuse(self->arena, Arena_get(new_msg_self->arena)); return new_msg; } diff --git a/upb/message/copy.c b/upb/message/copy.c index 83b1353f3a..a47ab65ac0 100644 --- a/upb/message/copy.c +++ b/upb/message/copy.c @@ -195,7 +195,7 @@ upb_Message* _upb_Message_Copy(upb_Message* dst, const upb_Message* src, upb_Arena* arena) { upb_StringView empty_string = upb_StringView_FromDataAndSize(NULL, 0); // Only copy message area skipping upb_Message_Internal. - memcpy(dst, src, mini_table->size); + memcpy(dst, src, mini_table->UPB_PRIVATE(size)); for (size_t i = 0; i < mini_table->UPB_PRIVATE(field_count); ++i) { const upb_MiniTableField* field = &mini_table->UPB_PRIVATE(fields)[i]; if (upb_MiniTableField_IsScalar(field)) { @@ -304,9 +304,23 @@ bool upb_Message_DeepCopy(upb_Message* dst, const upb_Message* src, // Deep clones a message using the provided target arena. // // Returns NULL on failure. -upb_Message* upb_Message_DeepClone(const upb_Message* message, - const upb_MiniTable* mini_table, - upb_Arena* arena) { - upb_Message* clone = upb_Message_New(mini_table, arena); - return _upb_Message_Copy(clone, message, mini_table, arena); +upb_Message* upb_Message_DeepClone(const upb_Message* msg, + const upb_MiniTable* m, upb_Arena* arena) { + upb_Message* clone = upb_Message_New(m, arena); + return _upb_Message_Copy(clone, msg, m, arena); +} + +// Performs a shallow copy. TODO: Extend to handle unknown fields. +void upb_Message_ShallowCopy(upb_Message* dst, const upb_Message* src, + const upb_MiniTable* m) { + memcpy(dst, src, m->UPB_PRIVATE(size)); +} + +// Performs a shallow clone. Ignores unknown fields. +upb_Message* upb_Message_ShallowClone(const upb_Message* msg, + const upb_MiniTable* m, + upb_Arena* arena) { + upb_Message* clone = upb_Message_New(m, arena); + upb_Message_ShallowCopy(clone, msg, m); + return clone; } diff --git a/upb/message/copy.h b/upb/message/copy.h index ce682efa53..17f82d85c7 100644 --- a/upb/message/copy.h +++ b/upb/message/copy.h @@ -21,9 +21,12 @@ extern "C" { #endif // Deep clones a message using the provided target arena. -upb_Message* upb_Message_DeepClone(const upb_Message* message, - const upb_MiniTable* mini_table, - upb_Arena* arena); +upb_Message* upb_Message_DeepClone(const upb_Message* msg, + const upb_MiniTable* m, upb_Arena* arena); + +// Shallow clones a message using the provided target arena. +upb_Message* upb_Message_ShallowClone(const upb_Message* msg, + const upb_MiniTable* m, upb_Arena* arena); // Deep clones array contents. upb_Array* upb_Array_DeepClone(const upb_Array* array, upb_CType value_type, @@ -37,7 +40,11 @@ upb_Map* upb_Map_DeepClone(const upb_Map* map, upb_CType key_type, // Deep copies the message from src to dst. bool upb_Message_DeepCopy(upb_Message* dst, const upb_Message* src, - const upb_MiniTable* mini_table, upb_Arena* arena); + const upb_MiniTable* m, upb_Arena* arena); + +// Shallow copies the message from src to dst. +void upb_Message_ShallowCopy(upb_Message* dst, const upb_Message* src, + const upb_MiniTable* m); #ifdef __cplusplus } /* extern "C" */ diff --git a/upb/message/internal/message.h b/upb/message/internal/message.h index c93ff783f9..216dc013c7 100644 --- a/upb/message/internal/message.h +++ b/upb/message/internal/message.h @@ -64,7 +64,7 @@ struct upb_Message_InternalData { }; UPB_INLINE size_t upb_msg_sizeof(const upb_MiniTable* m) { - return m->size + sizeof(upb_Message_Internal); + return m->UPB_PRIVATE(size) + sizeof(upb_Message_Internal); } // Inline version upb_Message_New(), for internal use. diff --git a/upb/mini_descriptor/decode.c b/upb/mini_descriptor/decode.c index 76fbeba701..1dafe0de1e 100644 --- a/upb/mini_descriptor/decode.c +++ b/upb/mini_descriptor/decode.c @@ -566,20 +566,21 @@ static void upb_MtDecoder_AssignHasbits(upb_MtDecoder* d) { } } - ret->size = last_hasbit ? upb_MiniTable_DivideRoundUp(last_hasbit + 1, 8) : 0; + ret->UPB_PRIVATE(size) = + last_hasbit ? upb_MiniTable_DivideRoundUp(last_hasbit + 1, 8) : 0; } size_t upb_MtDecoder_Place(upb_MtDecoder* d, upb_FieldRep rep) { size_t size = upb_MtDecoder_SizeOfRep(rep, d->platform); size_t align = upb_MtDecoder_AlignOfRep(rep, d->platform); - size_t ret = UPB_ALIGN_UP(d->table->size, align); + size_t ret = UPB_ALIGN_UP(d->table->UPB_PRIVATE(size), align); static const size_t max = UINT16_MAX; size_t new_size = ret + size; if (new_size > max) { upb_MdDecoder_ErrorJmp( &d->base, "Message size exceeded maximum size of %zu bytes", max); } - d->table->size = new_size; + d->table->UPB_PRIVATE(size) = new_size; return ret; } @@ -629,7 +630,7 @@ static void upb_MtDecoder_AssignOffsets(upb_MtDecoder* d) { // // On 32-bit we could potentially make this smaller, but there is no // compelling reason to optimize this right now. - d->table->size = UPB_ALIGN_UP(d->table->size, 8); + d->table->UPB_PRIVATE(size) = UPB_ALIGN_UP(d->table->UPB_PRIVATE(size), 8); } static void upb_MtDecoder_ValidateEntryField(upb_MtDecoder* d, @@ -690,7 +691,8 @@ static void upb_MtDecoder_ParseMap(upb_MtDecoder* d, const char* data, const size_t hasbit_size = 8; d->fields[0].offset = hasbit_size; d->fields[1].offset = hasbit_size + kv_size; - d->table->size = UPB_ALIGN_UP(hasbit_size + kv_size + kv_size, 8); + d->table->UPB_PRIVATE(size) = + UPB_ALIGN_UP(hasbit_size + kv_size + kv_size, 8); // Map entries have a special bit set to signal it's a map entry, used in // upb_MiniTable_SetSubMessage() below. @@ -705,7 +707,7 @@ static void upb_MtDecoder_ParseMessageSet(upb_MtDecoder* d, const char* data, } upb_MiniTable* ret = d->table; - ret->size = 0; + ret->UPB_PRIVATE(size) = 0; ret->UPB_PRIVATE(field_count) = 0; ret->UPB_PRIVATE(ext) = kUpb_ExtMode_IsMessageSet; ret->UPB_PRIVATE(dense_below) = 0; @@ -718,7 +720,7 @@ static upb_MiniTable* upb_MtDecoder_DoBuildMiniTableWithBuf( size_t* buf_size) { upb_MdDecoder_CheckOutOfMemory(&decoder->base, decoder->table); - decoder->table->size = 0; + decoder->table->UPB_PRIVATE(size) = 0; decoder->table->UPB_PRIVATE(field_count) = 0; decoder->table->UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable; decoder->table->UPB_PRIVATE(dense_below) = 0; diff --git a/upb/mini_descriptor/internal/encode_test.cc b/upb/mini_descriptor/internal/encode_test.cc index 8674cccde9..4984641060 100644 --- a/upb/mini_descriptor/internal/encode_test.cc +++ b/upb/mini_descriptor/internal/encode_test.cc @@ -71,7 +71,7 @@ TEST_P(MiniTableTest, AllScalarTypes) { EXPECT_EQ(i + 1, upb_MiniTableField_Number(f)); EXPECT_TRUE(upb_MiniTableField_IsScalar(f)); EXPECT_TRUE(offsets.insert(f->offset).second); - EXPECT_TRUE(f->offset < table->size); + EXPECT_TRUE(f->offset < table->UPB_PRIVATE(size)); } EXPECT_EQ(0, table->UPB_PRIVATE(required_count)); } @@ -97,7 +97,7 @@ TEST_P(MiniTableTest, AllRepeatedTypes) { EXPECT_EQ(i + 1, upb_MiniTableField_Number(f)); EXPECT_TRUE(upb_MiniTableField_IsArray(f)); EXPECT_TRUE(offsets.insert(f->offset).second); - EXPECT_TRUE(f->offset < table->size); + EXPECT_TRUE(f->offset < table->UPB_PRIVATE(size)); } EXPECT_EQ(0, table->UPB_PRIVATE(required_count)); } @@ -126,7 +126,7 @@ TEST_P(MiniTableTest, Skips) { EXPECT_EQ(kUpb_FieldType_Float, upb_MiniTableField_Type(f)); EXPECT_TRUE(upb_MiniTableField_IsScalar(f)); EXPECT_TRUE(offsets.insert(f->offset).second); - EXPECT_TRUE(f->offset < table->size); + EXPECT_TRUE(f->offset < table->UPB_PRIVATE(size)); } EXPECT_EQ(0, table->UPB_PRIVATE(required_count)); } @@ -159,8 +159,8 @@ TEST_P(MiniTableTest, AllScalarTypesOneof) { // All presence fields should point to the same oneof case offset. size_t case_ofs = _upb_MiniTableField_OneofOffset(f); EXPECT_EQ(table->UPB_PRIVATE(fields)[0].presence, f->presence); - EXPECT_TRUE(f->offset < table->size); - EXPECT_TRUE(case_ofs < table->size); + EXPECT_TRUE(f->offset < table->UPB_PRIVATE(size)); + EXPECT_TRUE(case_ofs < table->UPB_PRIVATE(size)); EXPECT_TRUE(case_ofs != f->offset); } EXPECT_EQ(0, table->UPB_PRIVATE(required_count)); diff --git a/upb/mini_table/internal/message.c b/upb/mini_table/internal/message.c index f1755f26d9..a1d736aa32 100644 --- a/upb/mini_table/internal/message.c +++ b/upb/mini_table/internal/message.c @@ -16,7 +16,7 @@ const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(subs) = NULL, .UPB_PRIVATE(fields) = NULL, - .size = 0, + .UPB_PRIVATE(size) = 0, .UPB_PRIVATE(field_count) = 0, .UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable, .UPB_PRIVATE(dense_below) = 0, diff --git a/upb/mini_table/internal/message.h b/upb/mini_table/internal/message.h index 95ce39d0f9..ff4e2b5f68 100644 --- a/upb/mini_table/internal/message.h +++ b/upb/mini_table/internal/message.h @@ -46,7 +46,7 @@ struct upb_MiniTable { // Must be aligned to sizeof(void*). Doesn't include internal members like // unknown fields, extension dict, pointer to msglayout, etc. - uint16_t size; + uint16_t UPB_PRIVATE(size); uint16_t UPB_ONLYBITS(field_count); diff --git a/upb/wire/decode_fast.c b/upb/wire/decode_fast.c index 34e6ede189..46c9371a7e 100644 --- a/upb/wire/decode_fast.c +++ b/upb/wire/decode_fast.c @@ -867,9 +867,9 @@ TAGBYTES(r) /* message fields *************************************************************/ UPB_INLINE -upb_Message* decode_newmsg_ceil(upb_Decoder* d, const upb_MiniTable* l, +upb_Message* decode_newmsg_ceil(upb_Decoder* d, const upb_MiniTable* m, int msg_ceil_bytes) { - size_t size = l->size + sizeof(upb_Message_Internal); + size_t size = m->UPB_PRIVATE(size) + sizeof(upb_Message_Internal); char* msg_data; if (UPB_LIKELY(msg_ceil_bytes > 0 && _upb_ArenaHas(&d->arena) >= msg_ceil_bytes)) { diff --git a/upb_generator/protoc-gen-upb.cc b/upb_generator/protoc-gen-upb.cc index 6397e16dbd..4fcfdabc81 100644 --- a/upb_generator/protoc-gen-upb.cc +++ b/upb_generator/protoc-gen-upb.cc @@ -942,8 +942,8 @@ void WriteHeader(const DefPoolPair& pools, upb::FileDefPtr file, size_t max64 = 0; for (const auto message : this_file_messages) { if (absl::EndsWith(message.name(), "Options")) { - size_t size32 = pools.GetMiniTable32(message)->size; - size_t size64 = pools.GetMiniTable64(message)->size; + size_t size32 = pools.GetMiniTable32(message)->UPB_PRIVATE(size); + size_t size64 = pools.GetMiniTable64(message)->UPB_PRIVATE(size); if (size32 > max32) { max32 = size32; max32_message = message; diff --git a/upb_generator/protoc-gen-upb_minitable.cc b/upb_generator/protoc-gen-upb_minitable.cc index eec387d294..1c5d636f2d 100644 --- a/upb_generator/protoc-gen-upb_minitable.cc +++ b/upb_generator/protoc-gen-upb_minitable.cc @@ -318,7 +318,7 @@ bool TryFillTableEntry(const DefPoolPair& pools, upb::FieldDefPtr field, // cross-file sub-message parsing if we are comfortable requiring that // users compile all messages at the same time. const upb_MiniTable* sub_mt = pools.GetMiniTable64(field.message_type()); - size = sub_mt->size + 8; + size = sub_mt->UPB_PRIVATE(size) + 8; } std::vector breaks = {64, 128, 192, 256}; for (auto brk : breaks) { @@ -474,7 +474,7 @@ void WriteMessage(upb::MessageDefPtr message, const DefPoolPair& pools, output(" $0,\n", submsgs_array_ref); output(" $0,\n", fields_array_ref); output(" $0, $1, $2, $3, UPB_FASTTABLE_MASK($4), $5,\n", - ArchDependentSize(mt_32->size, mt_64->size), + ArchDependentSize(mt_32->UPB_PRIVATE(size), mt_64->UPB_PRIVATE(size)), mt_64->UPB_PRIVATE(field_count), msgext, mt_64->UPB_PRIVATE(dense_below), table_mask, mt_64->UPB_PRIVATE(required_count));