From 895bca17c06c44b02726898698694e9d50ebbe2e Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Mon, 30 Sep 2019 16:54:16 +0000 Subject: [PATCH 01/10] Avoid initializing primitive fields in layout_init --- php/ext/google/protobuf/message.c | 3 ++- php/ext/google/protobuf/protobuf.h | 1 + php/ext/google/protobuf/storage.c | 26 +++++++++++++++++--------- php/tests/gdb_test.sh | 4 ++-- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 303f5d4741f..a616de40caf 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -255,7 +255,7 @@ void custom_data_init(const zend_class_entry* ce, MessageHeader* intern PHP_PROTO_TSRMLS_DC) { Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(ce)); intern->data = ALLOC_N(uint8_t, desc->layout->size); - memset(message_data(intern), 0, desc->layout->size); + memcpy(message_data(intern), desc->layout->empty_template, desc->layout->size); // We wrap first so that everything in the message object is GC-rooted in // case a collection happens during object creation in layout_init(). intern->descriptor = desc; @@ -541,6 +541,7 @@ PHP_METHOD(Message, clear) { zend_object_std_dtor(&msg->std TSRMLS_CC); object_properties_init(&msg->std, ce); + memcpy(message_data(msg), desc->layout->empty_template, desc->layout->size); layout_init(desc->layout, message_data(msg), &msg->std TSRMLS_CC); } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 1f1fb3ac8df..d3d66073236 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -935,6 +935,7 @@ struct MessageField { struct MessageLayout { const upb_msgdef* msgdef; + void* empty_template; // Can memcpy() onto a layout to clear it. MessageField* fields; size_t size; }; diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 1c28b1c1855..fdeca742b67 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -599,6 +599,8 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { // Reserve space for unknown fields. off += sizeof(void*); + layout->empty_template = NULL; + TSRMLS_FETCH(); Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msgdef)); layout->fields = ALLOC_N(MessageField, nfields); @@ -744,10 +746,15 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { layout->size = off; layout->msgdef = msgdef; + // Create the empty message template. + layout->empty_template = ALLOC_N(char, layout->size); + memset(layout->empty_template, 0, layout->size); + return layout; } void free_layout(MessageLayout* layout) { + FREE(layout->empty_template); FREE(layout->fields); FREE(layout); } @@ -757,21 +764,14 @@ void layout_init(MessageLayout* layout, void* storage, int i; upb_msg_field_iter it; - // Init unknown fields - memset(storage, 0, sizeof(void*)); - for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it); upb_msg_field_next(&it), i++) { const upb_fielddef* field = upb_msg_iter_field(&it); void* memory = slot_memory(layout, storage, field); - uint32_t* oneof_case = slot_oneof_case(layout, storage, field); int cache_index = slot_property_cache(layout, storage, field); CACHED_VALUE* property_ptr = OBJ_PROP(object, cache_index); - if (upb_fielddef_containingoneof(field)) { - memset(memory, 0, NATIVE_SLOT_MAX_SIZE); - *oneof_case = ONEOF_CASE_NONE; - } else if (is_map_field(field)) { + if (is_map_field(field)) { zval_ptr_dtor(property_ptr); #if PHP_MAJOR_VERSION < 7 MAKE_STD_ZVAL(*property_ptr); @@ -788,7 +788,15 @@ void layout_init(MessageLayout* layout, void* storage, property_ptr PHP_PROTO_TSRMLS_CC); DEREF(memory, CACHED_VALUE*) = property_ptr; } else { - native_slot_init(upb_fielddef_type(field), memory, property_ptr); + switch (upb_fielddef_type(field)) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + case UPB_TYPE_MESSAGE: + DEREF(memory, CACHED_VALUE*) = property_ptr; + break; + default: + break; + } } } } diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index da5f3f3ac14..cb336e30380 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -11,8 +11,8 @@ php -i | grep "Configuration" # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which # phpunit` --bootstrap autoload.php tmp_test.php # -# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php generated_class_test.php -gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php +gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php generated_class_test.php +# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php # # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # From ffd4beb41098b6741eec1784c50f81c0996e44ce Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 1 Oct 2019 01:08:31 +0000 Subject: [PATCH 02/10] Avoid initializing string/bytes/message fields in layout_init --- .gitignore | 2 + php/ext/google/protobuf/encode_decode.c | 155 +++++++++++++++++------- php/ext/google/protobuf/message.c | 8 +- php/ext/google/protobuf/protobuf.h | 4 +- php/ext/google/protobuf/storage.c | 120 +++++++++++++----- php/tests/gdb_test.sh | 4 +- 6 files changed, 207 insertions(+), 86 deletions(-) diff --git a/.gitignore b/.gitignore index dbbec532115..4e909aecea4 100644 --- a/.gitignore +++ b/.gitignore @@ -139,6 +139,8 @@ composer.lock php/tests/generated/ php/tests/old_protoc php/tests/protobuf/ +php/tests/core +php/tests/vgcore* php/ext/google/protobuf/.libs/ php/ext/google/protobuf/Makefile.fragments php/ext/google/protobuf/Makefile.global diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 166fb91baba..5d53408429d 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -118,6 +118,17 @@ static void stackenv_uninit(stackenv* se) { } } +// ----------------------------------------------------------------------------- +// Utility. +// ----------------------------------------------------------------------------- + +static CACHED_VALUE* find_cache(MessageHeader* msg, const upb_fielddef* field) { + int property_cache_index = + msg->descriptor->layout->fields[upb_fielddef_index(field)] + .cache_index; + return OBJ_PROP(&msg->std, property_cache_index); +} + // ----------------------------------------------------------------------------- // Parsing. // ----------------------------------------------------------------------------- @@ -139,6 +150,15 @@ static const void* newhandlerdata(upb_handlers* h, uint32_t ofs) { return hd_ofs; } +static const void* newhandlerfielddata( + upb_handlers* h, const upb_fielddef* field) { + void** hd_field = (void**)malloc(sizeof(void*)); + PHP_PROTO_ASSERT(hd_field != NULL); + *hd_field = field; + upb_handlers_addcleanup(h, hd_field, free); + return hd_field; +} + typedef struct { void* closure; stringsink sink; @@ -163,16 +183,18 @@ static const void *newunknownfieldshandlerdata(upb_handlers* h) { } typedef struct { + const upb_fielddef *fd; size_t ofs; const upb_msgdef *md; } submsg_handlerdata_t; -// Creates a handlerdata that contains offset and submessage type information. +// Creates a handlerdata that contains field and submessage type information. static const void *newsubmsghandlerdata(upb_handlers* h, uint32_t ofs, const upb_fielddef* f) { submsg_handlerdata_t* hd = (submsg_handlerdata_t*)malloc(sizeof(submsg_handlerdata_t)); PHP_PROTO_ASSERT(hd != NULL); + hd->fd = f; hd->ofs = ofs; hd->md = upb_fielddef_msgsubdef(f); upb_handlers_addcleanup(h, hd, free); @@ -322,15 +344,6 @@ static void *empty_php_string(zval* value_ptr) { } #endif #if PHP_MAJOR_VERSION < 7 -static void *empty_php_string2(zval** value_ptr) { - SEPARATE_ZVAL_IF_NOT_REF(value_ptr); - if (Z_TYPE_PP(value_ptr) == IS_STRING && - !IS_INTERNED(Z_STRVAL_PP(value_ptr))) { - FREE(Z_STRVAL_PP(value_ptr)); - } - ZVAL_EMPTY_STRING(*value_ptr); - return (void*)(*value_ptr); -} static void new_php_string(zval** value_ptr, const char* str, size_t len) { SEPARATE_ZVAL_IF_NOT_REF(value_ptr); if (Z_TYPE_PP(value_ptr) == IS_STRING && @@ -340,13 +353,6 @@ static void new_php_string(zval** value_ptr, const char* str, size_t len) { ZVAL_STRINGL(*value_ptr, str, len, 1); } #else -static void *empty_php_string2(zval* value_ptr) { - if (Z_TYPE_P(value_ptr) == IS_STRING) { - zend_string_release(Z_STR_P(value_ptr)); - } - ZVAL_EMPTY_STRING(value_ptr); - return value_ptr; -} static void new_php_string(zval* value_ptr, const char* str, size_t len) { if (Z_TYPE_P(value_ptr) == IS_STRING) { zend_string_release(Z_STR_P(value_ptr)); @@ -371,6 +377,21 @@ static void* str_handler(void *closure, } static bool str_end_handler(void *closure, const void *hd) { + stringfields_parseframe_t* frame = closure; + const upb_fielddef **field = hd; + MessageHeader* msg = (MessageHeader*)frame->closure; + + CACHED_VALUE* cached = find_cache(msg, *field); + + new_php_string(cached, frame->sink.ptr, frame->sink.len); + + stringsink_uninit(&frame->sink); + free(frame); + + return true; +} + +static bool map_str_end_handler(void *closure, const void *hd) { stringfields_parseframe_t* frame = closure; const size_t *ofs = hd; MessageHeader* msg = (MessageHeader*)frame->closure; @@ -430,26 +451,60 @@ static void *submsg_handler(void *closure, const void *hd) { zval* submsg_php; MessageHeader* submsg; - if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(message_data(msg), submsgdata->ofs, - CACHED_VALUE*))) == IS_NULL) { + CACHED_VALUE* cached = find_cache(msg, submsgdata->fd); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(cached)) == IS_NULL) { #if PHP_MAJOR_VERSION < 7 zval val; ZVAL_OBJ(&val, subklass->create_object(subklass TSRMLS_CC)); MessageHeader* intern = UNBOX(MessageHeader, &val); custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); - REPLACE_ZVAL_VALUE(DEREF(message_data(msg), submsgdata->ofs, zval**), - &val, 1); + REPLACE_ZVAL_VALUE(cached, &val, 1); zval_dtor(&val); #else zend_object* obj = subklass->create_object(subklass TSRMLS_CC); - ZVAL_OBJ(DEREF(message_data(msg), submsgdata->ofs, zval*), obj); + ZVAL_OBJ(cached, obj); MessageHeader* intern = UNBOX_HASHTABLE_VALUE(MessageHeader, obj); custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); #endif } - submsg_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), submsgdata->ofs, CACHED_VALUE*)); + submsg_php = CACHED_PTR_TO_ZVAL_PTR(cached); + + submsg = UNBOX(MessageHeader, submsg_php); + return submsg; +} + +static void *map_submsg_handler(void *closure, const void *hd) { + MessageHeader* msg = closure; + const submsg_handlerdata_t* submsgdata = hd; + TSRMLS_FETCH(); + Descriptor* subdesc = + UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)submsgdata->md)); + zend_class_entry* subklass = subdesc->klass; + zval* submsg_php; + MessageHeader* submsg; + + CACHED_VALUE* cached = + DEREF(message_data(msg), submsgdata->ofs, CACHED_VALUE*); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(cached)) == IS_NULL) { +#if PHP_MAJOR_VERSION < 7 + zval val; + ZVAL_OBJ(&val, subklass->create_object(subklass TSRMLS_CC)); + MessageHeader* intern = UNBOX(MessageHeader, &val); + custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); + REPLACE_ZVAL_VALUE(cached, &val, 1); + zval_dtor(&val); +#else + zend_object* obj = subklass->create_object(subklass TSRMLS_CC); + ZVAL_OBJ(cached, obj); + MessageHeader* intern = UNBOX_HASHTABLE_VALUE(MessageHeader, obj); + custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); +#endif + } + + submsg_php = CACHED_PTR_TO_ZVAL_PTR(cached); submsg = UNBOX(MessageHeader, submsg_php); return submsg; @@ -884,7 +939,7 @@ static void add_handlers_for_repeated_field(upb_handlers *h, // Set up handlers for a singular field. static void add_handlers_for_singular_field(upb_handlers *h, const upb_fielddef *f, - size_t offset) { + size_t offset, bool is_map) { switch (upb_fielddef_type(f)) { #define SET_HANDLER(utype, ltype) \ case utype: { \ @@ -908,16 +963,29 @@ static void add_handlers_for_singular_field(upb_handlers *h, case UPB_TYPE_STRING: case UPB_TYPE_BYTES: { upb_handlerattr attr = UPB_HANDLERATTR_INIT; - attr.handler_data = newhandlerdata(h, offset); + if (is_map) { + attr.handler_data = newhandlerdata(h, offset); + } else { + attr.handler_data = newhandlerfielddata(h, f); + } upb_handlers_setstartstr(h, f, str_handler, &attr); upb_handlers_setstring(h, f, stringdata_handler, &attr); - upb_handlers_setendstr(h, f, str_end_handler, &attr); + if (is_map) { + upb_handlers_setendstr(h, f, map_str_end_handler, &attr); + } else { + upb_handlers_setendstr(h, f, str_end_handler, &attr); + } break; } case UPB_TYPE_MESSAGE: { upb_handlerattr attr = UPB_HANDLERATTR_INIT; - attr.handler_data = newsubmsghandlerdata(h, offset, f); - upb_handlers_setstartsubmsg(h, f, submsg_handler, &attr); + if (is_map) { + attr.handler_data = newsubmsghandlerdata(h, offset, f); + upb_handlers_setstartsubmsg(h, f, map_submsg_handler, &attr); + } else { + attr.handler_data = newsubmsghandlerdata(h, 0, f); + upb_handlers_setstartsubmsg(h, f, submsg_handler, &attr); + } break; } } @@ -951,10 +1019,10 @@ static void add_handlers_for_mapentry(const upb_msgdef* msgdef, upb_handlers* h, add_handlers_for_singular_field(h, key_field, offsetof(map_parse_frame_data_t, - key_storage)); + key_storage), true); add_handlers_for_singular_field(h, value_field, offsetof(map_parse_frame_data_t, - value_storage)); + value_storage), true); } // Set up handlers for a oneof field. @@ -1063,7 +1131,7 @@ void add_handlers_for_message(const void* closure, upb_handlers* h) { } else if (upb_fielddef_isseq(f)) { add_handlers_for_repeated_field(h, f, offset); } else { - add_handlers_for_singular_field(h, f, offset); + add_handlers_for_singular_field(h, f, offset, false); } } } @@ -1102,6 +1170,8 @@ static void putjsonstruct( static void putstr(zval* str, const upb_fielddef* f, upb_sink sink, bool force_default); +static void putstr2(MessageHeader* msg, const upb_fielddef* f, upb_sink sink, + bool force_default); static void putrawstr(const char* str, int len, const upb_fielddef* f, upb_sink sink, bool force_default); @@ -1259,16 +1329,13 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, const upb_fielddef* type_field = upb_msgdef_itof(desc->msgdef, UPB_ANY_TYPE); const upb_fielddef* value_field = upb_msgdef_itof(desc->msgdef, UPB_ANY_VALUE); - uint32_t type_url_offset; zval* type_url_php_str; const upb_msgdef *payload_type = NULL; upb_sink_startmsg(sink); /* Handle type url */ - type_url_offset = desc->layout->fields[upb_fielddef_index(type_field)].offset; - type_url_php_str = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), type_url_offset, CACHED_VALUE*)); + type_url_php_str = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, type_field)); if (Z_STRLEN_P(type_url_php_str) > 0) { putstr(type_url_php_str, type_field, sink, false); } @@ -1294,14 +1361,11 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, } { - uint32_t value_offset; zval* value_php_str; const char* value_str; size_t value_len; - value_offset = desc->layout->fields[upb_fielddef_index(value_field)].offset; - value_php_str = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), value_offset, CACHED_VALUE*)); + value_php_str = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, value_field)); value_str = Z_STRVAL_P(value_php_str); value_len = Z_STRLEN_P(value_php_str); @@ -1467,16 +1531,14 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc, putarray(array, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isstring(f)) { - zval* str = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); + zval* str = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); if (containing_oneof || (is_json && is_wrapper_msg(desc->msgdef)) || Z_STRLEN_P(str) > 0) { putstr(str, f, sink, is_json && is_wrapper_msg(desc->msgdef)); } } else if (upb_fielddef_issubmsg(f)) { - putsubmsg(CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)), - f, sink, depth, is_json TSRMLS_CC); + zval* submsg = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + putsubmsg(submsg, f, sink, depth, is_json TSRMLS_CC); } else { upb_selector_t sel = getsel(f, upb_handlers_getprimitivehandlertype(f)); @@ -1890,8 +1952,7 @@ static void discard_unknown_fields(MessageHeader* msg) { discard_unknown_fields(submsg); } } else if (upb_fielddef_issubmsg(f)) { - zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); + zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); if (Z_TYPE_P(submsg_php) == IS_NULL) continue; MessageHeader* submsg = UNBOX(MessageHeader, submsg_php); discard_unknown_fields(submsg); diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index a616de40caf..5c5bd80f97b 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -178,7 +178,7 @@ static zval* message_get_property_internal(zval* object, zend_get_property_info(Z_OBJCE_P(object), Z_STR_P(member), true); #endif return layout_get( - self->descriptor->layout, message_data(self), field, + self->descriptor->layout, self, field, OBJ_PROP(Z_OBJ_P(object), property_info->offset) TSRMLS_CC); } @@ -191,7 +191,7 @@ static void message_get_oneof_property_internal(zval* object, zval* member, return; } - layout_get(self->descriptor->layout, message_data(self), field, + layout_get(self->descriptor->layout, self, field, ZVAL_PTR_TO_CACHED_PTR(return_value) TSRMLS_CC); } @@ -576,9 +576,9 @@ PHP_METHOD(Message, readOneof) { const upb_fielddef* field = upb_msgdef_itof(msg->descriptor->msgdef, index); // Unlike singular fields, oneof fields share cached property. So we cannot - // let lay_get modify the cached property. Instead, we pass in the return + // let layout_get modify the cached property. Instead, we pass in the return // value directly. - layout_get(msg->descriptor->layout, message_data(msg), field, + layout_get(msg->descriptor->layout, msg, field, ZVAL_PTR_TO_CACHED_PTR(return_value) TSRMLS_CC); } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index d3d66073236..a0548c296ad 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -200,7 +200,7 @@ #define CACHED_VALUE zval* #define CACHED_TO_ZVAL_PTR(VALUE) (VALUE) -#define CACHED_PTR_TO_ZVAL_PTR(VALUE) (*VALUE) +#define CACHED_PTR_TO_ZVAL_PTR(VALUE) (*(CACHED_VALUE*)(VALUE)) #define ZVAL_PTR_TO_CACHED_PTR(VALUE) (&VALUE) #define ZVAL_PTR_TO_CACHED_VALUE(VALUE) (VALUE) #define ZVAL_TO_CACHED_VALUE(VALUE) (&VALUE) @@ -949,7 +949,7 @@ PHP_PROTO_WRAP_OBJECT_END MessageLayout* create_layout(const upb_msgdef* msgdef); void layout_init(MessageLayout* layout, void* storage, zend_object* object PHP_PROTO_TSRMLS_DC); -zval* layout_get(MessageLayout* layout, const void* storage, +zval* layout_get(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, CACHED_VALUE* cache TSRMLS_DC); void layout_set(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, zval* val TSRMLS_DC); diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index fdeca742b67..33da4d0ca67 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -75,11 +75,9 @@ static bool native_slot_is_default(upb_fieldtype_t type, const void* memory) { #undef CASE_TYPE case UPB_TYPE_STRING: case UPB_TYPE_BYTES: - return Z_STRLEN_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(memory, CACHED_VALUE*))) == - 0; + return Z_STRLEN_P(CACHED_PTR_TO_ZVAL_PTR(memory)) == 0; case UPB_TYPE_MESSAGE: - return Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(memory, CACHED_VALUE*))) == - IS_NULL; + return Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(memory)) == IS_NULL; default: return false; } } @@ -787,16 +785,6 @@ void layout_init(MessageLayout* layout, void* storage, repeated_field_create_with_field(repeated_field_type, field, property_ptr PHP_PROTO_TSRMLS_CC); DEREF(memory, CACHED_VALUE*) = property_ptr; - } else { - switch (upb_fielddef_type(field)) { - case UPB_TYPE_STRING: - case UPB_TYPE_BYTES: - case UPB_TYPE_MESSAGE: - DEREF(memory, CACHED_VALUE*) = property_ptr; - break; - default: - break; - } } } } @@ -817,8 +805,61 @@ static void* value_memory(const upb_fielddef* field, void* memory) { return memory; } -zval* layout_get(MessageLayout* layout, const void* storage, +static void* value_memory2( + MessageHeader* header, const upb_fielddef* field, void* memory) { + switch (upb_fielddef_type(field)) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + case UPB_TYPE_MESSAGE: { + int property_cache_index = + header->descriptor->layout->fields[upb_fielddef_index(field)] + .cache_index; + return OBJ_PROP(&header->std, property_cache_index); + break; + } + default: + // No operation + break; + } + return memory; +} + +static void* value_memory3( + upb_fieldtype_t type, void* memory, CACHED_VALUE* cache) { + switch (type) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + case UPB_TYPE_MESSAGE: + return cache; + default: + // No operation + break; + } + return memory; +} + +static CACHED_VALUE* find_cache( + MessageHeader* header, const upb_fielddef* field) { + switch (upb_fielddef_type(field)) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + case UPB_TYPE_MESSAGE: { + int property_cache_index = + header->descriptor->layout->fields[upb_fielddef_index(field)] + .cache_index; + return OBJ_PROP(&header->std, property_cache_index); + break; + } + default: + // No operation + break; + } + return NULL; +} + +zval* layout_get(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, CACHED_VALUE* cache TSRMLS_DC) { + const void* storage = message_data(header); void* memory = slot_memory(layout, storage, field); uint32_t* oneof_case = slot_oneof_case(layout, storage, field); @@ -826,14 +867,17 @@ zval* layout_get(MessageLayout* layout, const void* storage, if (*oneof_case != upb_fielddef_number(field)) { native_slot_get_default(upb_fielddef_type(field), cache TSRMLS_CC); } else { - native_slot_get(upb_fielddef_type(field), value_memory(field, memory), - cache TSRMLS_CC); + upb_fieldtype_t type = upb_fielddef_type(field); + CACHED_VALUE* stored_cache = find_cache(header, field); + native_slot_get( + type, value_memory3(type, memory, stored_cache), cache TSRMLS_CC); } return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { return CACHED_PTR_TO_ZVAL_PTR(cache); } else { - native_slot_get(upb_fielddef_type(field), value_memory(field, memory), + upb_fieldtype_t type = upb_fielddef_type(field); + native_slot_get(type, value_memory3(type, memory, cache), cache TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } @@ -924,12 +968,16 @@ void layout_set(MessageLayout* layout, MessageHeader* header, Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg)); ce = desc->klass; } - native_slot_set(type, ce, value_memory(field, memory), val TSRMLS_CC); + CACHED_VALUE* cache = find_cache(header, field); + native_slot_set( + type, ce, value_memory3(upb_fielddef_type(field), memory, cache), + val TSRMLS_CC); } } -static void native_slot_merge(const upb_fielddef* field, const void* from_memory, - void* to_memory PHP_PROTO_TSRMLS_DC) { +static void native_slot_merge( + const upb_fielddef* field, const void* from_memory, + void* to_memory PHP_PROTO_TSRMLS_DC) { upb_fieldtype_t type = upb_fielddef_type(field); zend_class_entry* ce = NULL; if (!native_slot_is_default(type, from_memory)) { @@ -951,9 +999,8 @@ static void native_slot_merge(const upb_fielddef* field, const void* from_memory #undef CASE_TYPE case UPB_TYPE_STRING: case UPB_TYPE_BYTES: - native_slot_set(type, NULL, value_memory(field, to_memory), - CACHED_PTR_TO_ZVAL_PTR(DEREF( - from_memory, CACHED_VALUE*)) PHP_PROTO_TSRMLS_CC); + native_slot_set(type, NULL, to_memory, + CACHED_PTR_TO_ZVAL_PTR(from_memory) PHP_PROTO_TSRMLS_CC); break; case UPB_TYPE_MESSAGE: { const upb_msgdef* msg = upb_fielddef_msgsubdef(field); @@ -961,22 +1008,21 @@ static void native_slot_merge(const upb_fielddef* field, const void* from_memory ce = desc->klass; if (native_slot_is_default(type, to_memory)) { #if PHP_MAJOR_VERSION < 7 - SEPARATE_ZVAL_IF_NOT_REF((zval**)value_memory(field, to_memory)); + SEPARATE_ZVAL_IF_NOT_REF((zval**)to_memory); #endif CREATE_OBJ_ON_ALLOCATED_ZVAL_PTR( - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*)), ce); + CACHED_PTR_TO_ZVAL_PTR(to_memory), ce); MessageHeader* submsg = - UNBOX(MessageHeader, - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*))); + UNBOX(MessageHeader, CACHED_PTR_TO_ZVAL_PTR(to_memory)); custom_data_init(ce, submsg PHP_PROTO_TSRMLS_CC); } MessageHeader* sub_from = UNBOX(MessageHeader, - CACHED_PTR_TO_ZVAL_PTR(DEREF(from_memory, CACHED_VALUE*))); + CACHED_PTR_TO_ZVAL_PTR(from_memory)); MessageHeader* sub_to = UNBOX(MessageHeader, - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*))); + CACHED_PTR_TO_ZVAL_PTR(to_memory)); layout_merge(desc->layout, sub_from, sub_to PHP_PROTO_TSRMLS_CC); break; @@ -1137,7 +1183,19 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, } } } else { - native_slot_merge(field, from_memory, to_memory PHP_PROTO_TSRMLS_CC); + switch (upb_fielddef_type(field)) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + case UPB_TYPE_MESSAGE: { + CACHED_VALUE* from_cache = find_cache(from, field); + CACHED_VALUE* to_cache = find_cache(to, field); + native_slot_merge(field, from_cache, to_cache PHP_PROTO_TSRMLS_CC); + break; + } + default: + native_slot_merge(field, from_memory, to_memory PHP_PROTO_TSRMLS_CC); + break; + } } } } diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index cb336e30380..da5f3f3ac14 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -11,8 +11,8 @@ php -i | grep "Configuration" # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which # phpunit` --bootstrap autoload.php tmp_test.php # -gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php generated_class_test.php -# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php +# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php generated_class_test.php +gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php # # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # From 022ed2ab23b69a5aa62061a1dbcd7b11d8954974 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Oct 2019 16:48:56 +0000 Subject: [PATCH 03/10] Lazily create map when needed --- php/ext/google/protobuf/encode_decode.c | 41 ++++++++++---------- php/ext/google/protobuf/map.c | 12 ++++++ php/ext/google/protobuf/protobuf.h | 2 + php/ext/google/protobuf/storage.c | 50 +++++++++++-------------- 4 files changed, 57 insertions(+), 48 deletions(-) diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 5d53408429d..b0bd30eff1b 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -512,7 +512,7 @@ static void *map_submsg_handler(void *closure, const void *hd) { // Handler data for startmap/endmap handlers. typedef struct { - size_t ofs; + const upb_fielddef* fd; const upb_msgdef* value_md; upb_fieldtype_t key_field_type; upb_fieldtype_t value_field_type; @@ -667,9 +667,10 @@ static void map_slot_value(upb_fieldtype_t type, const void* from, static void *startmapentry_handler(void *closure, const void *hd) { MessageHeader* msg = closure; const map_handlerdata_t* mapdata = hd; + CACHED_VALUE* cache = find_cache(msg, mapdata->fd); TSRMLS_FETCH(); - zval* map = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), mapdata->ofs, CACHED_VALUE*)); + map_field_insure_created(mapdata->fd, cache PHP_PROTO_TSRMLS_CC); + zval* map = CACHED_PTR_TO_ZVAL_PTR(cache); map_parse_frame_t* frame = ALLOC(map_parse_frame_t); frame->data = ALLOC(map_parse_frame_data_t); @@ -717,7 +718,7 @@ static bool endmap_handler(void* closure, const void* hd, upb_status* s) { // key/value and endmsg handlers. The reason is that there is no easy way to // pass the handlerdata down to the sub-message handler setup. static map_handlerdata_t* new_map_handlerdata( - size_t ofs, + const upb_fielddef* field, const upb_msgdef* mapentry_def, Descriptor* desc) { const upb_fielddef* key_field; @@ -726,7 +727,7 @@ static map_handlerdata_t* new_map_handlerdata( map_handlerdata_t* hd = (map_handlerdata_t*)malloc(sizeof(map_handlerdata_t)); PHP_PROTO_ASSERT(hd != NULL); - hd->ofs = ofs; + hd->fd = field; key_field = upb_msgdef_itof(mapentry_def, MAP_KEY_FIELD); PHP_PROTO_ASSERT(key_field != NULL); hd->key_field_type = upb_fielddef_type(key_field); @@ -997,7 +998,7 @@ static void add_handlers_for_mapfield(upb_handlers* h, size_t offset, Descriptor* desc) { const upb_msgdef* map_msgdef = upb_fielddef_msgsubdef(fielddef); - map_handlerdata_t* hd = new_map_handlerdata(offset, map_msgdef, desc); + map_handlerdata_t* hd = new_map_handlerdata(fielddef, map_msgdef, desc); upb_handlerattr attr = UPB_HANDLERATTR_INIT; upb_handlers_addcleanup(h, hd, free); @@ -1448,16 +1449,20 @@ static void putjsonstruct( upb_sink_startmsg(sink); - map = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - intern = UNBOX(Map, map); - size = upb_strtable_count(&intern->table); - - if (size == 0) { + map = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + if (ZVAL_IS_NULL(map)) { upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); } else { - putmap(map, f, sink, depth, true TSRMLS_CC); + intern = UNBOX(Map, map); + size = upb_strtable_count(&intern->table); + + if (size == 0) { + upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); + upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); + } else { + putmap(map, f, sink, depth, true TSRMLS_CC); + } } upb_sink_endmsg(sink, &status); @@ -1519,9 +1524,8 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc, } if (is_map_field(f)) { - zval* map = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (map != NULL) { + zval* map = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + if (map != NULL && !ZVAL_IS_NULL(map)) { putmap(map, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isseq(f)) { @@ -1909,9 +1913,8 @@ static void discard_unknown_fields(MessageHeader* msg) { value_field = map_field_value(f); if (!upb_fielddef_issubmsg(value_field)) continue; - zval* map_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (map_php == NULL) continue; + zval* map_php = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + if (ZVAL_IS_NULL(map_php)) continue; Map* intern = UNBOX(Map, map_php); for (map_begin(map_php, &map_it TSRMLS_CC); diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index 0ce10190e6a..cd5c5016e8b 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -243,6 +243,18 @@ map_field_handlers->write_dimension = map_field_write_dimension; map_field_handlers->get_gc = map_field_get_gc; PHP_PROTO_INIT_CLASS_END +void map_field_insure_created(const upb_fielddef *field, + CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC) { + if (ZVAL_IS_NULL(CACHED_PTR_TO_ZVAL_PTR(map_field))) { + zval_ptr_dtor(map_field); +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(CACHED_PTR_TO_ZVAL_PTR(map_field)); +#endif + map_field_create_with_field(map_field_type, field, + map_field PHP_PROTO_TSRMLS_CC); + } +} + void map_field_create_with_field(const zend_class_entry *ce, const upb_fielddef *field, CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC) { diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index a0548c296ad..7e4d61d3928 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1090,6 +1090,8 @@ upb_value map_iter_value(MapIter* iter, int* len); const upb_fielddef* map_entry_key(const upb_msgdef* msgdef); const upb_fielddef* map_entry_value(const upb_msgdef* msgdef); +void map_field_insure_created(const upb_fielddef *field, + CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC); void map_field_create_with_field(const zend_class_entry* ce, const upb_fielddef* field, CACHED_VALUE* map_field PHP_PROTO_TSRMLS_DC); diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 33da4d0ca67..dcd7266ae8d 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -770,13 +770,6 @@ void layout_init(MessageLayout* layout, void* storage, CACHED_VALUE* property_ptr = OBJ_PROP(object, cache_index); if (is_map_field(field)) { - zval_ptr_dtor(property_ptr); -#if PHP_MAJOR_VERSION < 7 - MAKE_STD_ZVAL(*property_ptr); -#endif - map_field_create_with_field(map_field_type, field, - property_ptr PHP_PROTO_TSRMLS_CC); - DEREF(memory, CACHED_VALUE*) = property_ptr; } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { zval_ptr_dtor(property_ptr); #if PHP_MAJOR_VERSION < 7 @@ -840,21 +833,10 @@ static void* value_memory3( static CACHED_VALUE* find_cache( MessageHeader* header, const upb_fielddef* field) { - switch (upb_fielddef_type(field)) { - case UPB_TYPE_STRING: - case UPB_TYPE_BYTES: - case UPB_TYPE_MESSAGE: { - int property_cache_index = - header->descriptor->layout->fields[upb_fielddef_index(field)] - .cache_index; - return OBJ_PROP(&header->std, property_cache_index); - break; - } - default: - // No operation - break; - } - return NULL; + int property_cache_index = + header->descriptor->layout->fields[upb_fielddef_index(field)] + .cache_index; + return OBJ_PROP(&header->std, property_cache_index); } zval* layout_get(MessageLayout* layout, MessageHeader* header, @@ -873,6 +855,9 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header, type, value_memory3(type, memory, stored_cache), cache TSRMLS_CC); } return CACHED_PTR_TO_ZVAL_PTR(cache); + } else if (is_map_field(field)) { + map_field_insure_created(field, cache PHP_PROTO_TSRMLS_CC); + return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { return CACHED_PTR_TO_ZVAL_PTR(cache); } else { @@ -920,8 +905,8 @@ void layout_set(MessageLayout* layout, MessageHeader* header, *oneof_case = upb_fielddef_number(field); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { // Works for both repeated and map fields - memory = DEREF(memory, void**); - zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR((CACHED_VALUE*)memory); + CACHED_VALUE* cached = find_cache(header, field); + zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR(cached); if (EXPECTED(property_ptr != val)) { zend_class_entry *subce = NULL; @@ -953,7 +938,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, &converted_value); } #if PHP_MAJOR_VERSION < 7 - REPLACE_ZVAL_VALUE((zval**)memory, &converted_value, 1); + REPLACE_ZVAL_VALUE((zval**)cached, &converted_value, 1); #else php_proto_zval_ptr_dtor(property_ptr); ZVAL_ZVAL(property_ptr, &converted_value, 1, 0); @@ -1123,10 +1108,17 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, int size, key_length, value_length; MapIter map_it; - zval* to_map_php = - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*)); - zval* from_map_php = - CACHED_PTR_TO_ZVAL_PTR(DEREF(from_memory, CACHED_VALUE*)); + CACHED_VALUE* from_cache = find_cache(from, field); + CACHED_VALUE* to_cache = find_cache(to, field); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { + continue; + } + map_field_insure_created(field, to_cache PHP_PROTO_TSRMLS_CC); + + zval* to_map_php = CACHED_PTR_TO_ZVAL_PTR(to_cache); + zval* from_map_php = CACHED_PTR_TO_ZVAL_PTR(from_cache); + Map* to_map = UNBOX(Map, to_map_php); Map* from_map = UNBOX(Map, from_map_php); From 84eb2460061f62305ebe9051d0a95728c2f4ede2 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Oct 2019 17:31:42 +0000 Subject: [PATCH 04/10] Lazily create repeated fields --- php/ext/google/protobuf/array.c | 13 ++++++++ php/ext/google/protobuf/encode_decode.c | 40 ++++++++++++++----------- php/ext/google/protobuf/protobuf.h | 3 ++ php/ext/google/protobuf/storage.c | 20 +++++++------ 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index e69bef42998..8ac02441fa6 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -259,6 +259,19 @@ void repeated_field_push_native(RepeatedField *intern, void *value) { } } +void repeated_field_insure_created( + const upb_fielddef *field, + CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC) { + if (ZVAL_IS_NULL(CACHED_PTR_TO_ZVAL_PTR(repeated_field))) { + zval_ptr_dtor(repeated_field); +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(CACHED_PTR_TO_ZVAL_PTR(repeated_field)); +#endif + repeated_field_create_with_field(repeated_field_type, field, + repeated_field PHP_PROTO_TSRMLS_CC); + } +} + void repeated_field_create_with_field( zend_class_entry *ce, const upb_fielddef *field, CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC) { diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index b0bd30eff1b..f8e58125eef 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -243,8 +243,10 @@ static const void *newoneofhandlerdata(upb_handlers *h, // this field (such an instance always exists even in an empty message). static void *startseq_handler(void* closure, const void* hd) { MessageHeader* msg = closure; - const size_t *ofs = hd; - return CACHED_PTR_TO_ZVAL_PTR(DEREF(message_data(msg), *ofs, CACHED_VALUE*)); + const upb_fielddef** field = hd; + CACHED_VALUE* cache = find_cache(msg, *field); + repeated_field_insure_created(*field, cache PHP_PROTO_TSRMLS_CC); + return CACHED_PTR_TO_ZVAL_PTR(cache); } // Handlers that append primitive values to a repeated field. @@ -900,7 +902,7 @@ static void add_handlers_for_repeated_field(upb_handlers *h, const upb_fielddef *f, size_t offset) { upb_handlerattr attr = UPB_HANDLERATTR_INIT; - attr.handler_data = newhandlerdata(h, offset); + attr.handler_data = newhandlerfielddata(h, f); upb_handlers_setstartseq(h, f, startseq_handler, &attr); switch (upb_fielddef_type(f)) { @@ -1420,17 +1422,21 @@ static void putjsonlistvalue( upb_sink_startmsg(sink); - array = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - intern = UNBOX(RepeatedField, array); - ht = PHP_PROTO_HASH_OF(intern->array); - size = zend_hash_num_elements(ht); - - if (size == 0) { + array = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + if (ZVAL_IS_NULL(array)) { upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); } else { - putarray(array, f, sink, depth, true TSRMLS_CC); + intern = UNBOX(RepeatedField, array); + ht = PHP_PROTO_HASH_OF(intern->array); + size = zend_hash_num_elements(ht); + + if (size == 0) { + upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); + upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); + } else { + putarray(array, f, sink, depth, true TSRMLS_CC); + } } upb_sink_endmsg(sink, &status); @@ -1525,13 +1531,12 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc, if (is_map_field(f)) { zval* map = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); - if (map != NULL && !ZVAL_IS_NULL(map)) { + if (!ZVAL_IS_NULL(map)) { putmap(map, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isseq(f)) { - zval* array = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (array != NULL) { + zval* array = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + if (!ZVAL_IS_NULL(array)) { putarray(array, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isstring(f)) { @@ -1933,9 +1938,8 @@ static void discard_unknown_fields(MessageHeader* msg) { } else if (upb_fielddef_isseq(f)) { if (!upb_fielddef_issubmsg(f)) continue; - zval* array_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (array_php == NULL) continue; + zval* array_php = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + if (ZVAL_IS_NULL(array_php)) continue; int size, i; RepeatedField* intern = UNBOX(RepeatedField, array_php); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 7e4d61d3928..92b90a92044 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1150,6 +1150,9 @@ PHP_PROTO_WRAP_OBJECT_START(RepeatedFieldIter) long position; PHP_PROTO_WRAP_OBJECT_END +void repeated_field_insure_created( + const upb_fielddef *field, + CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC); void repeated_field_create_with_field( zend_class_entry* ce, const upb_fielddef* field, CACHED_VALUE* repeated_field PHP_PROTO_TSRMLS_DC); diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index dcd7266ae8d..8943c0fcda0 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -771,13 +771,6 @@ void layout_init(MessageLayout* layout, void* storage, if (is_map_field(field)) { } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - zval_ptr_dtor(property_ptr); -#if PHP_MAJOR_VERSION < 7 - MAKE_STD_ZVAL(*property_ptr); -#endif - repeated_field_create_with_field(repeated_field_type, field, - property_ptr PHP_PROTO_TSRMLS_CC); - DEREF(memory, CACHED_VALUE*) = property_ptr; } } } @@ -859,6 +852,7 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header, map_field_insure_created(field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { + repeated_field_insure_created(field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } else { upb_fieldtype_t type = upb_fielddef_type(field); @@ -1144,8 +1138,16 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, } } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - zval* to_array_php = CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*)); - zval* from_array_php = CACHED_PTR_TO_ZVAL_PTR(DEREF(from_memory, CACHED_VALUE*)); + CACHED_VALUE* from_cache = find_cache(from, field); + CACHED_VALUE* to_cache = find_cache(to, field); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { + continue; + } + repeated_field_insure_created(field, to_cache PHP_PROTO_TSRMLS_CC); + + zval* to_array_php = CACHED_PTR_TO_ZVAL_PTR(to_cache); + zval* from_array_php = CACHED_PTR_TO_ZVAL_PTR(from_cache); RepeatedField* to_array = UNBOX(RepeatedField, to_array_php); RepeatedField* from_array = UNBOX(RepeatedField, from_array_php); From dcc8581e372aeba7fcbd178c1767ec4de28dcce3 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Oct 2019 17:34:56 +0000 Subject: [PATCH 05/10] Change layout_init to only do memcpy --- php/ext/google/protobuf/message.c | 2 -- php/ext/google/protobuf/storage.c | 15 +-------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 5c5bd80f97b..03dec75928e 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -255,7 +255,6 @@ void custom_data_init(const zend_class_entry* ce, MessageHeader* intern PHP_PROTO_TSRMLS_DC) { Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(ce)); intern->data = ALLOC_N(uint8_t, desc->layout->size); - memcpy(message_data(intern), desc->layout->empty_template, desc->layout->size); // We wrap first so that everything in the message object is GC-rooted in // case a collection happens during object creation in layout_init(). intern->descriptor = desc; @@ -541,7 +540,6 @@ PHP_METHOD(Message, clear) { zend_object_std_dtor(&msg->std TSRMLS_CC); object_properties_init(&msg->std, ce); - memcpy(message_data(msg), desc->layout->empty_template, desc->layout->size); layout_init(desc->layout, message_data(msg), &msg->std TSRMLS_CC); } diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 8943c0fcda0..4ea555835bc 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -759,20 +759,7 @@ void free_layout(MessageLayout* layout) { void layout_init(MessageLayout* layout, void* storage, zend_object* object PHP_PROTO_TSRMLS_DC) { - int i; - upb_msg_field_iter it; - - for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it); - upb_msg_field_next(&it), i++) { - const upb_fielddef* field = upb_msg_iter_field(&it); - void* memory = slot_memory(layout, storage, field); - int cache_index = slot_property_cache(layout, storage, field); - CACHED_VALUE* property_ptr = OBJ_PROP(object, cache_index); - - if (is_map_field(field)) { - } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - } - } + memcpy(storage, layout->empty_template, layout->size); } // For non-singular fields, the related memory needs to point to the actual From 627d55ba8322bba8f736f9c87278d9b1e7792137 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Oct 2019 17:46:29 +0000 Subject: [PATCH 06/10] Fix test for php-7.0 --- php/ext/google/protobuf/protobuf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 92b90a92044..d6561267505 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -475,7 +475,7 @@ static inline int php_proto_zend_hash_get_current_data_ex(HashTable* ht, #define CACHED_VALUE zval #define CACHED_TO_ZVAL_PTR(VALUE) (&VALUE) -#define CACHED_PTR_TO_ZVAL_PTR(VALUE) (VALUE) +#define CACHED_PTR_TO_ZVAL_PTR(VALUE) ((CACHED_VALUE*)(VALUE)) #define ZVAL_PTR_TO_CACHED_PTR(VALUE) (VALUE) #define ZVAL_PTR_TO_CACHED_VALUE(VALUE) (*VALUE) #define ZVAL_TO_CACHED_VALUE(VALUE) (VALUE) From b54d22ed934c17d6b67a3d1336ec28557026efe0 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Oct 2019 21:53:46 +0000 Subject: [PATCH 07/10] Fix conformance test where default value of string/message map is not encoded --- php/ext/google/protobuf/encode_decode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index f8e58125eef..90aa60d192b 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -1228,7 +1228,7 @@ static void put_optional_value(const void* memory, int len, break; case UPB_TYPE_MESSAGE: { #if PHP_MAJOR_VERSION < 7 - MessageHeader *submsg = UNBOX(MessageHeader, *(zval**)memory); + MessageHeader *submsg = submsg = UNBOX(MessageHeader, *(zval**)memory); #else MessageHeader *submsg = (MessageHeader*)((char*)(*(zend_object**)memory) - From d8b1facca69816b471009e09fa75dcdc0f9ee439 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 2 Oct 2019 22:50:26 +0000 Subject: [PATCH 08/10] Fix test for zts --- php/ext/google/protobuf/encode_decode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 90aa60d192b..d9ecb4d5097 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -245,6 +245,7 @@ static void *startseq_handler(void* closure, const void* hd) { MessageHeader* msg = closure; const upb_fielddef** field = hd; CACHED_VALUE* cache = find_cache(msg, *field); + TSRMLS_FETCH(); repeated_field_insure_created(*field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } From e9012fc74accb0cf189cf39593f11e8a8eba5989 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Thu, 10 Oct 2019 18:07:16 +0000 Subject: [PATCH 09/10] Clean up --- php/ext/google/protobuf/encode_decode.c | 45 ++++++----------- php/ext/google/protobuf/protobuf.h | 3 ++ php/ext/google/protobuf/storage.c | 67 +++++++------------------ 3 files changed, 37 insertions(+), 78 deletions(-) diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index d9ecb4d5097..af3169c41f8 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -118,17 +118,6 @@ static void stackenv_uninit(stackenv* se) { } } -// ----------------------------------------------------------------------------- -// Utility. -// ----------------------------------------------------------------------------- - -static CACHED_VALUE* find_cache(MessageHeader* msg, const upb_fielddef* field) { - int property_cache_index = - msg->descriptor->layout->fields[upb_fielddef_index(field)] - .cache_index; - return OBJ_PROP(&msg->std, property_cache_index); -} - // ----------------------------------------------------------------------------- // Parsing. // ----------------------------------------------------------------------------- @@ -244,7 +233,7 @@ static const void *newoneofhandlerdata(upb_handlers *h, static void *startseq_handler(void* closure, const void* hd) { MessageHeader* msg = closure; const upb_fielddef** field = hd; - CACHED_VALUE* cache = find_cache(msg, *field); + CACHED_VALUE* cache = find_zval_property(msg, *field); TSRMLS_FETCH(); repeated_field_insure_created(*field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); @@ -384,7 +373,7 @@ static bool str_end_handler(void *closure, const void *hd) { const upb_fielddef **field = hd; MessageHeader* msg = (MessageHeader*)frame->closure; - CACHED_VALUE* cached = find_cache(msg, *field); + CACHED_VALUE* cached = find_zval_property(msg, *field); new_php_string(cached, frame->sink.ptr, frame->sink.len); @@ -454,7 +443,7 @@ static void *submsg_handler(void *closure, const void *hd) { zval* submsg_php; MessageHeader* submsg; - CACHED_VALUE* cached = find_cache(msg, submsgdata->fd); + CACHED_VALUE* cached = find_zval_property(msg, submsgdata->fd); if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(cached)) == IS_NULL) { #if PHP_MAJOR_VERSION < 7 @@ -670,7 +659,7 @@ static void map_slot_value(upb_fieldtype_t type, const void* from, static void *startmapentry_handler(void *closure, const void *hd) { MessageHeader* msg = closure; const map_handlerdata_t* mapdata = hd; - CACHED_VALUE* cache = find_cache(msg, mapdata->fd); + CACHED_VALUE* cache = find_zval_property(msg, mapdata->fd); TSRMLS_FETCH(); map_field_insure_created(mapdata->fd, cache PHP_PROTO_TSRMLS_CC); zval* map = CACHED_PTR_TO_ZVAL_PTR(cache); @@ -1174,8 +1163,6 @@ static void putjsonstruct( static void putstr(zval* str, const upb_fielddef* f, upb_sink sink, bool force_default); -static void putstr2(MessageHeader* msg, const upb_fielddef* f, upb_sink sink, - bool force_default); static void putrawstr(const char* str, int len, const upb_fielddef* f, upb_sink sink, bool force_default); @@ -1229,7 +1216,7 @@ static void put_optional_value(const void* memory, int len, break; case UPB_TYPE_MESSAGE: { #if PHP_MAJOR_VERSION < 7 - MessageHeader *submsg = submsg = UNBOX(MessageHeader, *(zval**)memory); + MessageHeader *submsg = UNBOX(MessageHeader, *(zval**)memory); #else MessageHeader *submsg = (MessageHeader*)((char*)(*(zend_object**)memory) - @@ -1339,7 +1326,7 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, upb_sink_startmsg(sink); /* Handle type url */ - type_url_php_str = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, type_field)); + type_url_php_str = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, type_field)); if (Z_STRLEN_P(type_url_php_str) > 0) { putstr(type_url_php_str, type_field, sink, false); } @@ -1369,7 +1356,7 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, const char* value_str; size_t value_len; - value_php_str = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, value_field)); + value_php_str = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, value_field)); value_str = Z_STRVAL_P(value_php_str); value_len = Z_STRLEN_P(value_php_str); @@ -1423,7 +1410,7 @@ static void putjsonlistvalue( upb_sink_startmsg(sink); - array = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + array = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (ZVAL_IS_NULL(array)) { upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); @@ -1456,7 +1443,7 @@ static void putjsonstruct( upb_sink_startmsg(sink); - map = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + map = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (ZVAL_IS_NULL(map)) { upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); @@ -1531,23 +1518,23 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc, } if (is_map_field(f)) { - zval* map = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* map = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (!ZVAL_IS_NULL(map)) { putmap(map, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isseq(f)) { - zval* array = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* array = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (!ZVAL_IS_NULL(array)) { putarray(array, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isstring(f)) { - zval* str = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* str = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (containing_oneof || (is_json && is_wrapper_msg(desc->msgdef)) || Z_STRLEN_P(str) > 0) { putstr(str, f, sink, is_json && is_wrapper_msg(desc->msgdef)); } } else if (upb_fielddef_issubmsg(f)) { - zval* submsg = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* submsg = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); putsubmsg(submsg, f, sink, depth, is_json TSRMLS_CC); } else { upb_selector_t sel = getsel(f, upb_handlers_getprimitivehandlertype(f)); @@ -1919,7 +1906,7 @@ static void discard_unknown_fields(MessageHeader* msg) { value_field = map_field_value(f); if (!upb_fielddef_issubmsg(value_field)) continue; - zval* map_php = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* map_php = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (ZVAL_IS_NULL(map_php)) continue; Map* intern = UNBOX(Map, map_php); @@ -1939,7 +1926,7 @@ static void discard_unknown_fields(MessageHeader* msg) { } else if (upb_fielddef_isseq(f)) { if (!upb_fielddef_issubmsg(f)) continue; - zval* array_php = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* array_php = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (ZVAL_IS_NULL(array_php)) continue; int size, i; @@ -1960,7 +1947,7 @@ static void discard_unknown_fields(MessageHeader* msg) { discard_unknown_fields(submsg); } } else if (upb_fielddef_issubmsg(f)) { - zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR(find_cache(msg, f)); + zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (Z_TYPE_P(submsg_php) == IS_NULL) continue; MessageHeader* submsg = UNBOX(MessageHeader, submsg_php); discard_unknown_fields(submsg); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index d6561267505..be955e1d379 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1495,6 +1495,9 @@ size_t stringsink_string(void *_sink, const void *hd, const char *ptr, #define FREE(object) efree(object) #define PEFREE(object) pefree(object, 1) +// Find corresponding zval property for the field. +CACHED_VALUE* find_zval_property(MessageHeader* msg, const upb_fielddef* field); + // String argument. #define STR(str) (str), strlen(str) diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 4ea555835bc..e9766d0b39f 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -762,42 +762,11 @@ void layout_init(MessageLayout* layout, void* storage, memcpy(storage, layout->empty_template, layout->size); } -// For non-singular fields, the related memory needs to point to the actual -// zval in properties table first. -static void* value_memory(const upb_fielddef* field, void* memory) { - switch (upb_fielddef_type(field)) { - case UPB_TYPE_STRING: - case UPB_TYPE_BYTES: - case UPB_TYPE_MESSAGE: - memory = DEREF(memory, CACHED_VALUE*); - break; - default: - // No operation - break; - } - return memory; -} - -static void* value_memory2( - MessageHeader* header, const upb_fielddef* field, void* memory) { - switch (upb_fielddef_type(field)) { - case UPB_TYPE_STRING: - case UPB_TYPE_BYTES: - case UPB_TYPE_MESSAGE: { - int property_cache_index = - header->descriptor->layout->fields[upb_fielddef_index(field)] - .cache_index; - return OBJ_PROP(&header->std, property_cache_index); - break; - } - default: - // No operation - break; - } - return memory; -} - -static void* value_memory3( +// Switch memory for processing for singular fields based on field type. +// * primitive fields: memory +// * others (string, bytes and message): cache (the correspond zval +// property) +static void* value_memory( upb_fieldtype_t type, void* memory, CACHED_VALUE* cache) { switch (type) { case UPB_TYPE_STRING: @@ -811,7 +780,7 @@ static void* value_memory3( return memory; } -static CACHED_VALUE* find_cache( +CACHED_VALUE* find_zval_property( MessageHeader* header, const upb_fielddef* field) { int property_cache_index = header->descriptor->layout->fields[upb_fielddef_index(field)] @@ -830,9 +799,9 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header, native_slot_get_default(upb_fielddef_type(field), cache TSRMLS_CC); } else { upb_fieldtype_t type = upb_fielddef_type(field); - CACHED_VALUE* stored_cache = find_cache(header, field); + CACHED_VALUE* stored_cache = find_zval_property(header, field); native_slot_get( - type, value_memory3(type, memory, stored_cache), cache TSRMLS_CC); + type, value_memory(type, memory, stored_cache), cache TSRMLS_CC); } return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (is_map_field(field)) { @@ -843,7 +812,7 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header, return CACHED_PTR_TO_ZVAL_PTR(cache); } else { upb_fieldtype_t type = upb_fielddef_type(field); - native_slot_get(type, value_memory3(type, memory, cache), + native_slot_get(type, value_memory(type, memory, cache), cache TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } @@ -886,7 +855,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, *oneof_case = upb_fielddef_number(field); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { // Works for both repeated and map fields - CACHED_VALUE* cached = find_cache(header, field); + CACHED_VALUE* cached = find_zval_property(header, field); zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR(cached); if (EXPECTED(property_ptr != val)) { @@ -934,9 +903,9 @@ void layout_set(MessageLayout* layout, MessageHeader* header, Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg)); ce = desc->klass; } - CACHED_VALUE* cache = find_cache(header, field); + CACHED_VALUE* cache = find_zval_property(header, field); native_slot_set( - type, ce, value_memory3(upb_fielddef_type(field), memory, cache), + type, ce, value_memory(upb_fielddef_type(field), memory, cache), val TSRMLS_CC); } } @@ -1089,8 +1058,8 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, int size, key_length, value_length; MapIter map_it; - CACHED_VALUE* from_cache = find_cache(from, field); - CACHED_VALUE* to_cache = find_cache(to, field); + CACHED_VALUE* from_cache = find_zval_property(from, field); + CACHED_VALUE* to_cache = find_zval_property(to, field); if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { continue; @@ -1125,8 +1094,8 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, } } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - CACHED_VALUE* from_cache = find_cache(from, field); - CACHED_VALUE* to_cache = find_cache(to, field); + CACHED_VALUE* from_cache = find_zval_property(from, field); + CACHED_VALUE* to_cache = find_zval_property(to, field); if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { continue; @@ -1168,8 +1137,8 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, case UPB_TYPE_STRING: case UPB_TYPE_BYTES: case UPB_TYPE_MESSAGE: { - CACHED_VALUE* from_cache = find_cache(from, field); - CACHED_VALUE* to_cache = find_cache(to, field); + CACHED_VALUE* from_cache = find_zval_property(from, field); + CACHED_VALUE* to_cache = find_zval_property(to, field); native_slot_merge(field, from_cache, to_cache PHP_PROTO_TSRMLS_CC); break; } From edc31999699abd211c55b59bc6d7e78b85b1a027 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 29 Oct 2019 17:54:21 +0000 Subject: [PATCH 10/10] Fix comments --- php/ext/google/protobuf/array.c | 2 +- php/ext/google/protobuf/encode_decode.c | 10 +++++----- php/ext/google/protobuf/map.c | 2 +- php/ext/google/protobuf/protobuf.h | 4 ++-- php/ext/google/protobuf/storage.c | 8 ++++---- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index 8ac02441fa6..b52bdf62ca7 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -259,7 +259,7 @@ void repeated_field_push_native(RepeatedField *intern, void *value) { } } -void repeated_field_insure_created( +void repeated_field_ensure_created( const upb_fielddef *field, CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC) { if (ZVAL_IS_NULL(CACHED_PTR_TO_ZVAL_PTR(repeated_field))) { diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index af3169c41f8..39304a57113 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -141,7 +141,7 @@ static const void* newhandlerdata(upb_handlers* h, uint32_t ofs) { static const void* newhandlerfielddata( upb_handlers* h, const upb_fielddef* field) { - void** hd_field = (void**)malloc(sizeof(void*)); + const void** hd_field = malloc(sizeof(void*)); PHP_PROTO_ASSERT(hd_field != NULL); *hd_field = field; upb_handlers_addcleanup(h, hd_field, free); @@ -232,10 +232,10 @@ static const void *newoneofhandlerdata(upb_handlers *h, // this field (such an instance always exists even in an empty message). static void *startseq_handler(void* closure, const void* hd) { MessageHeader* msg = closure; - const upb_fielddef** field = hd; + const upb_fielddef** field = (const upb_fielddef**) hd; CACHED_VALUE* cache = find_zval_property(msg, *field); TSRMLS_FETCH(); - repeated_field_insure_created(*field, cache PHP_PROTO_TSRMLS_CC); + repeated_field_ensure_created(*field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } @@ -370,7 +370,7 @@ static void* str_handler(void *closure, static bool str_end_handler(void *closure, const void *hd) { stringfields_parseframe_t* frame = closure; - const upb_fielddef **field = hd; + const upb_fielddef **field = (const upb_fielddef **) hd; MessageHeader* msg = (MessageHeader*)frame->closure; CACHED_VALUE* cached = find_zval_property(msg, *field); @@ -661,7 +661,7 @@ static void *startmapentry_handler(void *closure, const void *hd) { const map_handlerdata_t* mapdata = hd; CACHED_VALUE* cache = find_zval_property(msg, mapdata->fd); TSRMLS_FETCH(); - map_field_insure_created(mapdata->fd, cache PHP_PROTO_TSRMLS_CC); + map_field_ensure_created(mapdata->fd, cache PHP_PROTO_TSRMLS_CC); zval* map = CACHED_PTR_TO_ZVAL_PTR(cache); map_parse_frame_t* frame = ALLOC(map_parse_frame_t); diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index cd5c5016e8b..2764788b469 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -243,7 +243,7 @@ map_field_handlers->write_dimension = map_field_write_dimension; map_field_handlers->get_gc = map_field_get_gc; PHP_PROTO_INIT_CLASS_END -void map_field_insure_created(const upb_fielddef *field, +void map_field_ensure_created(const upb_fielddef *field, CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC) { if (ZVAL_IS_NULL(CACHED_PTR_TO_ZVAL_PTR(map_field))) { zval_ptr_dtor(map_field); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index be955e1d379..2301d1e2b02 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1090,7 +1090,7 @@ upb_value map_iter_value(MapIter* iter, int* len); const upb_fielddef* map_entry_key(const upb_msgdef* msgdef); const upb_fielddef* map_entry_value(const upb_msgdef* msgdef); -void map_field_insure_created(const upb_fielddef *field, +void map_field_ensure_created(const upb_fielddef *field, CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC); void map_field_create_with_field(const zend_class_entry* ce, const upb_fielddef* field, @@ -1150,7 +1150,7 @@ PHP_PROTO_WRAP_OBJECT_START(RepeatedFieldIter) long position; PHP_PROTO_WRAP_OBJECT_END -void repeated_field_insure_created( +void repeated_field_ensure_created( const upb_fielddef *field, CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC); void repeated_field_create_with_field( diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index e9766d0b39f..e6050d0eb3e 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -805,10 +805,10 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header, } return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (is_map_field(field)) { - map_field_insure_created(field, cache PHP_PROTO_TSRMLS_CC); + map_field_ensure_created(field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - repeated_field_insure_created(field, cache PHP_PROTO_TSRMLS_CC); + repeated_field_ensure_created(field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } else { upb_fieldtype_t type = upb_fielddef_type(field); @@ -1064,7 +1064,7 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { continue; } - map_field_insure_created(field, to_cache PHP_PROTO_TSRMLS_CC); + map_field_ensure_created(field, to_cache PHP_PROTO_TSRMLS_CC); zval* to_map_php = CACHED_PTR_TO_ZVAL_PTR(to_cache); zval* from_map_php = CACHED_PTR_TO_ZVAL_PTR(from_cache); @@ -1100,7 +1100,7 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { continue; } - repeated_field_insure_created(field, to_cache PHP_PROTO_TSRMLS_CC); + repeated_field_ensure_created(field, to_cache PHP_PROTO_TSRMLS_CC); zval* to_array_php = CACHED_PTR_TO_ZVAL_PTR(to_cache); zval* from_array_php = CACHED_PTR_TO_ZVAL_PTR(from_cache);