From 0eaeec8909dda5c3c96772eb1a7ab4007f076a21 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 00:41:48 +0000 Subject: [PATCH 01/16] Make reserve names map persistent --- php/ext/google/protobuf/protobuf.c | 61 +++++++++++++++++++++--------- php/ext/google/protobuf/protobuf.h | 14 +++++-- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 19cc5efb4ee7..175dbd899563 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -32,7 +32,6 @@ #include -ZEND_DECLARE_MODULE_GLOBALS(protobuf) static PHP_GINIT_FUNCTION(protobuf); static PHP_GSHUTDOWN_FUNCTION(protobuf); static PHP_RINIT_FUNCTION(protobuf); @@ -49,7 +48,11 @@ static HashTable* ce_to_php_obj_map; // Global map from message/enum's proto fully-qualified name to corresponding // wrapper Descriptor/EnumDescriptor instances. static HashTable* proto_to_php_obj_map; -static HashTable* reserved_names; + +// HashTable upb_def_to_php_obj_map_persistent; +// HashTable ce_to_php_obj_map_persistent; +// HashTable proto_to_php_obj_map_persistent; +upb_strtable reserved_names; // ----------------------------------------------------------------------------- // Global maps. @@ -184,9 +187,11 @@ const char *const kReservedNames[] = { const int kReservedNamesSize = 73; bool is_reserved_name(const char* name) { - void** value; - return (php_proto_zend_hash_find(reserved_names, name, strlen(name), - (void**)&value) == SUCCESS); + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_UINT64; +#endif + return upb_strtable_lookup2(&reserved_names, name, strlen(name), &v); } // ----------------------------------------------------------------------------- @@ -241,11 +246,13 @@ static void php_proto_hashtable_descriptor_release(zval* value) { } efree(ptr); } + +static void test_release(void* value) { + void* ptr = value; +} #endif static PHP_RINIT_FUNCTION(protobuf) { - int i = 0; - ALLOC_HASHTABLE(upb_def_to_php_obj_map); zend_hash_init(upb_def_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); @@ -255,13 +262,6 @@ static PHP_RINIT_FUNCTION(protobuf) { ALLOC_HASHTABLE(proto_to_php_obj_map); zend_hash_init(proto_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); - ALLOC_HASHTABLE(reserved_names); - zend_hash_init(reserved_names, 16, NULL, NULL, 0); - for (i = 0; i < kReservedNamesSize; i++) { - php_proto_zend_hash_update(reserved_names, kReservedNames[i], - strlen(kReservedNames[i])); - } - generated_pool = NULL; generated_pool_php = NULL; internal_generated_pool_php = NULL; @@ -290,9 +290,6 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { zend_hash_destroy(proto_to_php_obj_map); FREE_HASHTABLE(proto_to_php_obj_map); - zend_hash_destroy(reserved_names); - FREE_HASHTABLE(reserved_names); - #if PHP_MAJOR_VERSION < 7 if (generated_pool_php != NULL) { zval_dtor(generated_pool_php); @@ -329,7 +326,29 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { return 0; } +static void reserved_names_init() { + size_t i; + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_UINT64; +#endif + for (i = 0; i < kReservedNamesSize; i++) { + upb_strtable_insert2(&reserved_names, kReservedNames[i], + strlen(kReservedNames[i]), v); + } +} + static PHP_MINIT_FUNCTION(protobuf) { + // zend_hash_init(&upb_def_to_php_obj_map_persistent, 16, NULL, + // EG(persistent_list).pDestructor, 1); + // zend_hash_init(&ce_to_php_obj_map_persistent, 16, NULL, + // EG(persistent_list).pDestructor, 1); + // zend_hash_init(&proto_to_php_obj_map_persistent, 16, NULL, + // EG(persistent_list).pDestructor, 1); + upb_strtable_init(&reserved_names, UPB_CTYPE_UINT64); + + reserved_names_init(); + descriptor_pool_init(TSRMLS_C); descriptor_init(TSRMLS_C); enum_descriptor_init(TSRMLS_C); @@ -391,6 +410,14 @@ static PHP_MINIT_FUNCTION(protobuf) { } static PHP_MSHUTDOWN_FUNCTION(protobuf) { + // zend_hash_clean(&upb_def_to_php_obj_map_persistent); + // zend_hash_clean(&ce_to_php_obj_map_persistent); + // zend_hash_clean(&proto_to_php_obj_map_persistent); + // zend_hash_destory(&upb_def_to_php_obj_map_persistent); + // zend_hash_destory(&ce_to_php_obj_map_persistent); + // zend_hash_destory(&proto_to_php_obj_map_persistent); + upb_strtable_uninit(&reserved_names); + PEFREE(message_handlers); PEFREE(repeated_field_handlers); PEFREE(repeated_field_iter_handlers); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 6151ff4cd613..b6a1878ace05 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -171,7 +171,7 @@ LOWWERNAME##_methods); \ LOWWERNAME##_type = zend_register_internal_class(&class_type TSRMLS_CC); \ LOWWERNAME##_type->create_object = LOWWERNAME##_create; \ - LOWWERNAME##_handlers = PEMALLOC(zend_object_handlers); \ + LOWWERNAME##_handlers = PEMALLOC(zend_object_handlers, 1); \ memcpy(LOWWERNAME##_handlers, zend_get_std_object_handlers(), \ sizeof(zend_object_handlers)); #define PHP_PROTO_INIT_CLASS_END \ @@ -440,7 +440,7 @@ static inline int php_proto_zend_hash_get_current_data_ex(HashTable* ht, LOWWERNAME##_methods); \ LOWWERNAME##_type = zend_register_internal_class(&class_type TSRMLS_CC); \ LOWWERNAME##_type->create_object = LOWWERNAME##_create; \ - LOWWERNAME##_handlers = PEMALLOC(zend_object_handlers); \ + LOWWERNAME##_handlers = PEMALLOC(zend_object_handlers, 1); \ memcpy(LOWWERNAME##_handlers, zend_get_std_object_handlers(), \ sizeof(zend_object_handlers)); \ LOWWERNAME##_handlers->free_obj = LOWWERNAME##_free; \ @@ -682,6 +682,14 @@ typedef struct Value Value; ZEND_BEGIN_MODULE_GLOBALS(protobuf) ZEND_END_MODULE_GLOBALS(protobuf) +ZEND_DECLARE_MODULE_GLOBALS(protobuf) + +#ifdef ZTS +#define PROTOBUF_G(v) TSRMG(protobuf_globals_id, zend_protobuf_globals *, v) +#else +#define PROTOBUF_G(v) (protobuf_globals.v) +#endif + // Init module and PHP classes. void any_init(TSRMLS_D); void api_init(TSRMLS_D); @@ -1492,7 +1500,7 @@ size_t stringsink_string(void *_sink, const void *hd, const char *ptr, // Memory management #define ALLOC(class_name) (class_name*) emalloc(sizeof(class_name)) -#define PEMALLOC(class_name) (class_name*) pemalloc(sizeof(class_name), 1) +#define PEMALLOC(class_name, persistent) (class_name*) pemalloc(sizeof(class_name), persistent) #define ALLOC_N(class_name, n) (class_name*) emalloc(sizeof(class_name) * n) #define FREE(object) efree(object) #define PEFREE(object) pefree(object, 1) From 190c62e5e87e1fbe52bda92fcb4fa1f0bd03c8e6 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 02:05:52 +0000 Subject: [PATCH 02/16] Add DescriptorInternal to map --- php/ext/google/protobuf/def.c | 5 +++ php/ext/google/protobuf/protobuf.c | 49 +++++++++++++++++------------- php/ext/google/protobuf/protobuf.h | 13 ++++++++ 3 files changed, 46 insertions(+), 21 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 5df680248916..cdc3d89da6be 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -951,7 +951,12 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, CREATE_HASHTABLE_VALUE(desc, desc_php, Descriptor, descriptor_type); desc->msgdef = msgdef; desc->pool = pool; + desc->intern = SYS_MALLOC(DescriptorInternal); + desc->intern->msgdef = msgdef; + desc->intern->pool = pool; + add_def_obj(desc->msgdef, desc_php); + add_msgdef_desc(desc->msgdef, desc->intern); // Unlike other messages, MapEntry is shared by all map fields and doesn't // have generated PHP class. diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 175dbd899563..1f9d26bc0c35 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -49,9 +49,9 @@ static HashTable* ce_to_php_obj_map; // wrapper Descriptor/EnumDescriptor instances. static HashTable* proto_to_php_obj_map; -// HashTable upb_def_to_php_obj_map_persistent; -// HashTable ce_to_php_obj_map_persistent; -// HashTable proto_to_php_obj_map_persistent; +upb_inttable upb_def_to_php_obj_map_persistent; +upb_inttable ce_to_php_obj_map_persistent; +upb_inttable proto_to_php_obj_map_persistent; upb_strtable reserved_names; // ----------------------------------------------------------------------------- @@ -81,12 +81,6 @@ static bool exist_in_table(const HashTable* t, const void* def) { (void**)&value) == SUCCESS); } -static void add_to_list(HashTable* t, void* value) { - zval* pDest = NULL; - php_proto_zend_hash_next_index_insert_mem(t, &value, sizeof(void*), - (void**)&pDest); -} - static void add_to_strtable(HashTable* t, const char* key, int key_size, void* value) { zval* pDest = NULL; @@ -112,6 +106,10 @@ void add_def_obj(const void* def, PHP_PROTO_HASHTABLE_VALUE value) { add_to_table(upb_def_to_php_obj_map, def, value); } +void add_msgdef_desc(const upb_msgdef* m, DescriptorInternal* desc) { + upb_inttable_insertptr(&upb_def_to_php_obj_map_persistent, m, upb_value_ptr(desc)); +} + PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def) { return (PHP_PROTO_HASHTABLE_VALUE)get_from_table(upb_def_to_php_obj_map, def); } @@ -339,12 +337,9 @@ static void reserved_names_init() { } static PHP_MINIT_FUNCTION(protobuf) { - // zend_hash_init(&upb_def_to_php_obj_map_persistent, 16, NULL, - // EG(persistent_list).pDestructor, 1); - // zend_hash_init(&ce_to_php_obj_map_persistent, 16, NULL, - // EG(persistent_list).pDestructor, 1); - // zend_hash_init(&proto_to_php_obj_map_persistent, 16, NULL, - // EG(persistent_list).pDestructor, 1); + upb_inttable_init(&upb_def_to_php_obj_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&ce_to_php_obj_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&proto_to_php_obj_map_persistent, UPB_CTYPE_PTR); upb_strtable_init(&reserved_names, UPB_CTYPE_UINT64); reserved_names_init(); @@ -409,13 +404,25 @@ static PHP_MINIT_FUNCTION(protobuf) { return 0; } +static void cleanup_inttable(upb_inttable* t) { + upb_inttable_iter i; + upb_value v; + void* ptr; + for(upb_inttable_begin(&i, t); + !upb_inttable_done(&i); + upb_inttable_next(&i)) { + v = upb_inttable_iter_value(&i); + ptr = upb_value_getptr(v); + SYS_FREE(ptr); + } +} + static PHP_MSHUTDOWN_FUNCTION(protobuf) { - // zend_hash_clean(&upb_def_to_php_obj_map_persistent); - // zend_hash_clean(&ce_to_php_obj_map_persistent); - // zend_hash_clean(&proto_to_php_obj_map_persistent); - // zend_hash_destory(&upb_def_to_php_obj_map_persistent); - // zend_hash_destory(&ce_to_php_obj_map_persistent); - // zend_hash_destory(&proto_to_php_obj_map_persistent); + cleanup_inttable(&upb_def_to_php_obj_map_persistent); + + upb_inttable_uninit(&upb_def_to_php_obj_map_persistent); + upb_inttable_uninit(&ce_to_php_obj_map_persistent); + upb_inttable_uninit(&proto_to_php_obj_map_persistent); upb_strtable_uninit(&reserved_names); PEFREE(message_handlers); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index b6a1878ace05..3b04abc36b55 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -590,6 +590,7 @@ struct Api; struct BoolValue; struct BytesValue; struct Descriptor; +struct DescriptorInternal; struct DescriptorPool; struct DoubleValue; struct Duration; @@ -635,6 +636,7 @@ typedef struct Api Api; typedef struct BoolValue BoolValue; typedef struct BytesValue BytesValue; typedef struct Descriptor Descriptor; +typedef struct DescriptorInternal DescriptorInternal; typedef struct DescriptorPool DescriptorPool; typedef struct DoubleValue DoubleValue; typedef struct Duration Duration; @@ -751,6 +753,7 @@ void gpb_metadata_wrappers_init(TSRMLS_D); // instances. void add_def_obj(const void* def, PHP_PROTO_HASHTABLE_VALUE value); PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def); +void add_msgdef_desc(const upb_msgdef* m, DescriptorInternal* desc); // Global map from PHP class entries to wrapper Descriptor/EnumDescriptor // instances. @@ -811,11 +814,19 @@ void internal_descriptor_pool_free(zend_object* object); #endif extern InternalDescriptorPool* generated_pool; // The actual generated pool +struct DescriptorInternal { + InternalDescriptorPool* pool; + const upb_msgdef* msgdef; + MessageLayout* layout; + zend_class_entry* klass; // begins as NULL +}; + PHP_PROTO_WRAP_OBJECT_START(Descriptor) InternalDescriptorPool* pool; const upb_msgdef* msgdef; MessageLayout* layout; zend_class_entry* klass; // begins as NULL + DescriptorInternal* intern; PHP_PROTO_WRAP_OBJECT_END PHP_METHOD(Descriptor, getClass); @@ -1499,6 +1510,8 @@ size_t stringsink_string(void *_sink, const void *hd, const char *ptr, // ----------------------------------------------------------------------------- // Memory management +#define SYS_MALLOC(class_name) (class_name*) malloc(sizeof(class_name)) +#define SYS_FREE(ptr) free(ptr) #define ALLOC(class_name) (class_name*) emalloc(sizeof(class_name)) #define PEMALLOC(class_name, persistent) (class_name*) pemalloc(sizeof(class_name), persistent) #define ALLOC_N(class_name, n) (class_name*) emalloc(sizeof(class_name) * n) From f69b6c6e86c5cc670ed60b9772cb8d207af25159 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 03:03:19 +0000 Subject: [PATCH 03/16] Use get_msgdef_desc in encode_decode.c --- php/ext/google/protobuf/def.c | 1 + php/ext/google/protobuf/encode_decode.c | 107 +++++++++++------------- php/ext/google/protobuf/message.c | 10 +-- php/ext/google/protobuf/protobuf.c | 12 +++ php/ext/google/protobuf/protobuf.h | 3 +- 5 files changed, 66 insertions(+), 67 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index cdc3d89da6be..4460843297d3 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -966,6 +966,7 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, desc->klass = register_class(file, upb_msgdef_fullname(msgdef), desc_php, use_nested_submsg TSRMLS_CC); + desc->intern->klass = desc->klass; if (desc->klass == NULL) { return; diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 2612d22d6696..f9128b125437 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -428,8 +428,7 @@ static void *appendsubmsg_handler(void *closure, const void *hd) { RepeatedField* intern = UNBOX(RepeatedField, array); const submsg_handlerdata_t *submsgdata = hd; - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)submsgdata->md)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdata->md); zend_class_entry* subklass = subdesc->klass; MessageHeader* submsg; @@ -456,8 +455,7 @@ static void *appendwrappersubmsg_handler(void *closure, const void *hd) { RepeatedField* intern = UNBOX(RepeatedField, array); const submsg_handlerdata_t *submsgdata = hd; - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)submsgdata->md)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdata->md); zend_class_entry* subklass = subdesc->klass; MessageHeader* submsg; wrapperfields_parseframe_t* frame = @@ -488,8 +486,7 @@ static void *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)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdata->md); zend_class_entry* subklass = subdesc->klass; zval* submsg_php; MessageHeader* submsg; @@ -522,8 +519,7 @@ 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)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdata->md); zend_class_entry* subklass = subdesc->klass; zval* submsg_php; MessageHeader* submsg; @@ -557,8 +553,7 @@ static void *map_wrapper_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)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdata->md); zend_class_entry* subklass = subdesc->klass; zval* submsg_php; MessageHeader* submsg; @@ -645,8 +640,7 @@ static void map_slot_init( break; } case UPB_TYPE_MESSAGE: { - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(value_msg)); + DescriptorInternal* subdesc = get_msgdef_desc(value_msg); zend_class_entry* subklass = subdesc->klass; MessageHeader* submsg; #if PHP_MAJOR_VERSION < 7 @@ -802,8 +796,7 @@ static bool endmap_handler(void* closure, const void* hd, upb_status* s) { // pass the handlerdata down to the sub-message handler setup. static map_handlerdata_t* new_map_handlerdata( const upb_fielddef* field, - const upb_msgdef* mapentry_def, - Descriptor* desc) { + const upb_msgdef* mapentry_def) { const upb_fielddef* key_field; const upb_fielddef* value_field; // TODO(teboring): Use emalloc and efree. @@ -944,8 +937,7 @@ static void* oneofsubmsg_handler(void* closure, const void* hd) { const oneof_handlerdata_t *oneofdata = hd; uint32_t oldcase = DEREF(message_data(msg), oneofdata->case_ofs, uint32_t); TSRMLS_FETCH(); - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)oneofdata->md)); + DescriptorInternal* subdesc = get_msgdef_desc(oneofdata->md); zend_class_entry* subklass = subdesc->klass; zval* submsg_php; MessageHeader* submsg; @@ -983,8 +975,7 @@ static void* wrapper_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)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdata->md); zend_class_entry* subklass = subdesc->klass; zval* submsg_php; MessageHeader* submsg; @@ -1015,8 +1006,7 @@ static void* wrapper_oneofsubmsg_handler(void* closure, const void* hd) { const oneof_handlerdata_t *oneofdata = hd; uint32_t oldcase = DEREF(message_data(msg), oneofdata->case_ofs, uint32_t); TSRMLS_FETCH(); - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)oneofdata->md)); + DescriptorInternal* subdesc = get_msgdef_desc(oneofdata->md); zend_class_entry* subklass = subdesc->klass; wrapperfields_parseframe_t* frame = (wrapperfields_parseframe_t*)malloc(sizeof(wrapperfields_parseframe_t)); @@ -1164,10 +1154,9 @@ static void add_handlers_for_singular_field(upb_handlers *h, // Adds handlers to a map field. static void add_handlers_for_mapfield(upb_handlers* h, const upb_fielddef* fielddef, - size_t offset, - Descriptor* desc) { + size_t offset) { const upb_msgdef* map_msgdef = upb_fielddef_msgsubdef(fielddef); - map_handlerdata_t* hd = new_map_handlerdata(fielddef, map_msgdef, desc); + map_handlerdata_t* hd = new_map_handlerdata(fielddef, map_msgdef); upb_handlerattr attr = UPB_HANDLERATTR_INIT; upb_handlers_addcleanup(h, hd, free); @@ -1176,11 +1165,11 @@ static void add_handlers_for_mapfield(upb_handlers* h, } // Adds handlers to a map-entry msgdef. -static void add_handlers_for_mapentry(const upb_msgdef* msgdef, upb_handlers* h, - Descriptor* desc) { +static void add_handlers_for_mapentry( + const upb_msgdef* msgdef, upb_handlers* h) { const upb_fielddef* key_field = map_entry_key(msgdef); const upb_fielddef* value_field = map_entry_value(msgdef); - map_handlerdata_t* hd = new_map_handlerdata(0, msgdef, desc); + map_handlerdata_t* hd = new_map_handlerdata(0, msgdef); upb_handlerattr attr = UPB_HANDLERATTR_INIT; upb_handlers_addcleanup(h, hd, free); @@ -1294,11 +1283,11 @@ static bool strwrapper_end_handler(void *closure, const void *hd) { static void add_handlers_for_wrapper(const upb_msgdef* msgdef, upb_handlers* h) { const upb_fielddef* f = upb_msgdef_itof(msgdef, 1); - Descriptor* desc; + DescriptorInternal* desc; size_t offset; TSRMLS_FETCH(); - desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)msgdef)); + desc = get_msgdef_desc(msgdef); offset = desc->layout->fields[upb_fielddef_index(f)].offset; switch (upb_msgdef_wellknowntype(msgdef)) { @@ -1357,14 +1346,13 @@ static bool add_unknown_handler(void* closure, const void* hd, const char* buf, void add_handlers_for_message(const void* closure, upb_handlers* h) { const upb_msgdef* msgdef = upb_handlers_msgdef(h); TSRMLS_FETCH(); - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)msgdef)); + DescriptorInternal* desc = get_msgdef_desc(msgdef); upb_msg_field_iter i; // If this is a mapentry message type, set up a special set of handlers and // bail out of the normal (user-defined) message type handling. if (upb_msgdef_mapentry(msgdef)) { - add_handlers_for_mapentry(msgdef, h, desc); + add_handlers_for_mapentry(msgdef, h); return; } @@ -1402,7 +1390,7 @@ void add_handlers_for_message(const void* closure, upb_handlers* h) { add_handlers_for_oneof_field(h, desc->msgdef, f, offset, oneof_case_offset, property_cache_index); } else if (is_map_field(f)) { - add_handlers_for_mapfield(h, f, offset, desc); + add_handlers_for_mapfield(h, f, offset); } else if (upb_fielddef_isseq(f)) { add_handlers_for_repeated_field(h, f, offset); } else { @@ -1413,15 +1401,15 @@ void add_handlers_for_message(const void* closure, upb_handlers* h) { // Constructs the handlers for filling a message's data into an in-memory // object. -const upb_handlers* get_fill_handlers(Descriptor* desc) { +const upb_handlers* get_fill_handlers(DescriptorInternal* desc) { return upb_handlercache_get(desc->pool->fill_handler_cache, desc->msgdef); } -static const upb_pbdecodermethod *msgdef_decodermethod(Descriptor* desc) { +static const upb_pbdecodermethod *msgdef_decodermethod(DescriptorInternal* desc) { return upb_pbcodecache_get(desc->pool->fill_method_cache, desc->msgdef); } -static const upb_json_parsermethod *msgdef_jsonparsermethod(Descriptor* desc) { +static const upb_json_parsermethod *msgdef_jsonparsermethod(DescriptorInternal* desc) { return upb_json_codecache_get(desc->pool->json_fill_method_cache, desc->msgdef); } @@ -1429,21 +1417,21 @@ static const upb_json_parsermethod *msgdef_jsonparsermethod(Descriptor* desc) { // Serializing. // ----------------------------------------------------------------------------- -static void putmsg(zval* msg, const Descriptor* desc, upb_sink sink, +static void putmsg(zval* msg, const DescriptorInternal* desc, upb_sink sink, int depth, bool is_json TSRMLS_DC); -static void putrawmsg(MessageHeader* msg, const Descriptor* desc, +static void putrawmsg(MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth, bool is_json, bool open_msg TSRMLS_DC); static void putwrappervalue( zval* value, const upb_fielddef* f, upb_sink sink, int depth, bool is_json TSRMLS_DC); -static void putjsonany(MessageHeader* msg, const Descriptor* desc, +static void putjsonany(MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth TSRMLS_DC); static void putjsonlistvalue( - MessageHeader* msg, const Descriptor* desc, + MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth TSRMLS_DC); static void putjsonstruct( - MessageHeader* msg, const Descriptor* desc, + MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth TSRMLS_DC); static void putstr(zval* str, const upb_fielddef* f, upb_sink sink, @@ -1590,16 +1578,16 @@ static void putmap(zval* map, const upb_fielddef* f, upb_sink sink, upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); } -static void putmsg(zval* msg_php, const Descriptor* desc, upb_sink sink, +static void putmsg(zval* msg_php, const DescriptorInternal* desc, upb_sink sink, int depth, bool is_json TSRMLS_DC) { MessageHeader* msg = UNBOX(MessageHeader, msg_php); putrawmsg(msg, desc, sink, depth, is_json, true TSRMLS_CC); } static const upb_handlers* msgdef_json_serialize_handlers( - Descriptor* desc, bool preserve_proto_fieldnames); + DescriptorInternal* desc, bool preserve_proto_fieldnames); -static void putjsonany(MessageHeader* msg, const Descriptor* desc, +static void putjsonany(MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth TSRMLS_DC) { upb_status status; const upb_fielddef* type_field = upb_msgdef_itof(desc->msgdef, UPB_ANY_TYPE); @@ -1646,8 +1634,7 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, value_len = Z_STRLEN_P(value_php_str); if (value_len > 0) { - Descriptor* payload_desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)payload_type)); + DescriptorInternal* payload_desc = get_msgdef_desc(payload_type); zend_class_entry* payload_klass = payload_desc->klass; zval val; upb_sink subsink; @@ -1682,7 +1669,7 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, } static void putjsonlistvalue( - MessageHeader* msg, const Descriptor* desc, + MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth TSRMLS_DC) { upb_status status; upb_sink subsink; @@ -1716,7 +1703,7 @@ static void putjsonlistvalue( } static void putjsonstruct( - MessageHeader* msg, const Descriptor* desc, + MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth TSRMLS_DC) { upb_status status; upb_sink subsink; @@ -1747,7 +1734,7 @@ static void putjsonstruct( upb_sink_endmsg(sink, &status); } -static void putrawmsg(MessageHeader* msg, const Descriptor* desc, +static void putrawmsg(MessageHeader* msg, const DescriptorInternal* desc, upb_sink sink, int depth, bool is_json, bool open_msg TSRMLS_DC) { upb_msg_field_iter i; @@ -1976,8 +1963,8 @@ static void putrawsubmsg(MessageHeader* submsg, const upb_fielddef* f, upb_sink sink, int depth, bool is_json TSRMLS_DC) { upb_sink subsink; - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(upb_fielddef_msgsubdef(f))); + const upb_msgdef* m = upb_fielddef_msgsubdef(f); + DescriptorInternal* subdesc = get_msgdef_desc(m); upb_sink_startsubmsg(sink, getsel(f, UPB_HANDLER_STARTSUBMSG), &subsink); putrawmsg(submsg, subdesc, subsink, depth + 1, is_json, true TSRMLS_CC); @@ -2051,13 +2038,13 @@ static void putarray(zval* array, const upb_fielddef* f, upb_sink sink, upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); } -static const upb_handlers* msgdef_pb_serialize_handlers(Descriptor* desc) { +static const upb_handlers* msgdef_pb_serialize_handlers(DescriptorInternal* desc) { return upb_handlercache_get(desc->pool->pb_serialize_handler_cache, desc->msgdef); } static const upb_handlers* msgdef_json_serialize_handlers( - Descriptor* desc, bool preserve_proto_fieldnames) { + DescriptorInternal* desc, bool preserve_proto_fieldnames) { if (preserve_proto_fieldnames) { return upb_handlercache_get( desc->pool->json_serialize_handler_preserve_cache, desc->msgdef); @@ -2079,7 +2066,7 @@ void serialize_to_string(zval* val, zval* return_value TSRMLS_DC) { stringsink_init(&sink); { - const upb_handlers* serialize_handlers = msgdef_pb_serialize_handlers(desc); + const upb_handlers* serialize_handlers = msgdef_pb_serialize_handlers(desc->intern); stackenv se; upb_pb_encoder* encoder; @@ -2087,7 +2074,7 @@ void serialize_to_string(zval* val, zval* return_value TSRMLS_DC) { stackenv_init(&se, "Error occurred during encoding: %s"); encoder = upb_pb_encoder_create(se.arena, serialize_handlers, sink.sink); - putmsg(val, desc, upb_pb_encoder_input(encoder), 0, false TSRMLS_CC); + putmsg(val, desc->intern, upb_pb_encoder_input(encoder), 0, false TSRMLS_CC); PHP_PROTO_RETVAL_STRINGL(sink.ptr, sink.len, 1); @@ -2100,7 +2087,7 @@ PHP_METHOD(Message, serializeToString) { serialize_to_string(getThis(), return_value TSRMLS_CC); } -void merge_from_string(const char* data, int data_len, Descriptor* desc, +void merge_from_string(const char* data, int data_len, DescriptorInternal* desc, MessageHeader* msg) { const upb_pbdecodermethod* method = msgdef_decodermethod(desc); const upb_handlers* h = upb_pbdecodermethod_desthandlers(method); @@ -2145,7 +2132,7 @@ PHP_METHOD(Message, mergeFromString) { return; } - merge_from_string(data, data_len, desc, msg); + merge_from_string(data, data_len, desc->intern, msg); } PHP_METHOD(Message, serializeToJsonString) { @@ -2163,14 +2150,14 @@ PHP_METHOD(Message, serializeToJsonString) { { const upb_handlers* serialize_handlers = - msgdef_json_serialize_handlers(desc, preserve_proto_fieldnames); + msgdef_json_serialize_handlers(desc->intern, preserve_proto_fieldnames); upb_json_printer* printer; stackenv se; stackenv_init(&se, "Error occurred during encoding: %s"); printer = upb_json_printer_create(se.arena, serialize_handlers, sink.sink); - putmsg(getThis(), desc, upb_json_printer_input(printer), 0, true TSRMLS_CC); + putmsg(getThis(), desc->intern, upb_json_printer_input(printer), 0, true TSRMLS_CC); PHP_PROTO_RETVAL_STRINGL(sink.ptr, sink.len, 1); @@ -2202,7 +2189,7 @@ PHP_METHOD(Message, mergeFromJsonString) { // TODO(teboring): Clear message. { - const upb_json_parsermethod* method = msgdef_jsonparsermethod(desc); + const upb_json_parsermethod* method = msgdef_jsonparsermethod(desc->intern); stackenv se; upb_sink sink; upb_json_parser* parser; @@ -2220,7 +2207,7 @@ PHP_METHOD(Message, mergeFromJsonString) { closure = msg; } - upb_sink_reset(&sink, get_fill_handlers(desc), closure); + upb_sink_reset(&sink, get_fill_handlers(desc->intern), closure); parser = upb_json_parser_create(se.arena, method, generated_pool->symtab, sink, &se.status, ignore_json_unknown); upb_bufsrc_putbuf(data, data_len, upb_json_parser_input(parser)); diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 291c2e5dfef6..40e3a18450d2 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -288,6 +288,7 @@ void build_class_from_descriptor( if (desc->layout == NULL) { MessageLayout* layout = create_layout(desc->msgdef); desc->layout = layout; + desc->intern->layout = layout; } registered_ce->create_object = message_create; @@ -395,8 +396,7 @@ void Message_construct(zval* msg, zval* array_wrapper) { is_wrapper = is_wrapper_msg(submsgdef); if (is_wrapper) { - PHP_PROTO_HASHTABLE_VALUE subdesc_php = get_def_obj(submsgdef); - Descriptor* subdesc = UNBOX_HASHTABLE_VALUE(Descriptor, subdesc_php); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdef); subklass = subdesc->klass; } } @@ -435,8 +435,7 @@ void Message_construct(zval* msg, zval* array_wrapper) { is_wrapper = is_wrapper_msg(submsgdef); if (is_wrapper) { - PHP_PROTO_HASHTABLE_VALUE subdesc_php = get_def_obj(submsgdef); - Descriptor* subdesc = UNBOX_HASHTABLE_VALUE(Descriptor, subdesc_php); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdef); subklass = subdesc->klass; } } @@ -459,8 +458,7 @@ void Message_construct(zval* msg, zval* array_wrapper) { } } else if (upb_fielddef_issubmsg(field)) { const upb_msgdef* submsgdef = upb_fielddef_msgsubdef(field); - PHP_PROTO_HASHTABLE_VALUE desc_php = get_def_obj(submsgdef); - Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); + DescriptorInternal* desc = get_msgdef_desc(submsgdef); CACHED_VALUE* cached = NULL; if (upb_fielddef_containingoneof(field)) { diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 1f9d26bc0c35..c6a4ed6de437 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -110,6 +110,18 @@ void add_msgdef_desc(const upb_msgdef* m, DescriptorInternal* desc) { upb_inttable_insertptr(&upb_def_to_php_obj_map_persistent, m, upb_value_ptr(desc)); } +DescriptorInternal* get_msgdef_desc(const upb_msgdef* m) { + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_PTR; +#endif + if (!upb_inttable_lookupptr(&upb_def_to_php_obj_map_persistent, m, &v)) { + return NULL; + } else { + return upb_value_getptr(v); + } +} + PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def) { return (PHP_PROTO_HASHTABLE_VALUE)get_from_table(upb_def_to_php_obj_map, def); } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 3b04abc36b55..cf9444cc1dc4 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -754,6 +754,7 @@ void gpb_metadata_wrappers_init(TSRMLS_D); void add_def_obj(const void* def, PHP_PROTO_HASHTABLE_VALUE value); PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def); void add_msgdef_desc(const upb_msgdef* m, DescriptorInternal* desc); +DescriptorInternal* get_msgdef_desc(const upb_msgdef* m); // Global map from PHP class entries to wrapper Descriptor/EnumDescriptor // instances. @@ -1004,7 +1005,7 @@ PHP_METHOD(Message, __construct); const upb_pbdecodermethod *new_fillmsg_decodermethod(Descriptor *desc, const void *owner); void serialize_to_string(zval* val, zval* return_value TSRMLS_DC); -void merge_from_string(const char* data, int data_len, Descriptor* desc, +void merge_from_string(const char* data, int data_len, DescriptorInternal* desc, MessageHeader* msg); PHP_METHOD(Message, serializeToString); From 87c9f0501c97c0c87ef826901011e916bc61f211 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 19:21:18 +0000 Subject: [PATCH 04/16] Add persistent map for ce=>def and enum=>def --- php/ext/google/protobuf/def.c | 20 +++- php/ext/google/protobuf/message.c | 2 +- php/ext/google/protobuf/protobuf.c | 142 ++++++++++++++++++++++++++--- php/ext/google/protobuf/protobuf.h | 18 ++++ php/ext/google/protobuf/storage.c | 21 ++--- php/tests/gdb_test.sh | 3 +- 6 files changed, 173 insertions(+), 33 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 4460843297d3..9c74a30daffa 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -837,7 +837,8 @@ static void fill_classname(const char *fullname, static zend_class_entry *register_class(const upb_filedef *file, const char *fullname, PHP_PROTO_HASHTABLE_VALUE desc_php, - bool use_nested_submsg TSRMLS_DC) { + bool use_nested_submsg, + bool is_enum TSRMLS_DC) { // Prepend '.' to package name to make it absolute. In the 5 additional // bytes allocated, one for '.', one for trailing 0, and 3 for 'GPB' if // given message is google.protobuf.Empty. @@ -867,6 +868,15 @@ static zend_class_entry *register_class(const upb_filedef *file, ret = PHP_PROTO_CE_UNREF(pce); add_ce_obj(ret, desc_php); add_proto_obj(fullname, desc_php); + if (is_enum) { + EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE(EnumDescriptor, desc_php); + add_ce_enumdesc(ret, desc->intern); + add_proto_enumdesc(fullname, desc->intern); + } else { + Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); + add_ce_desc(ret, desc->intern); + add_proto_desc(fullname, desc->intern); + } stringsink_uninit(&namesink); return ret; } @@ -965,7 +975,7 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, } desc->klass = register_class(file, upb_msgdef_fullname(msgdef), desc_php, - use_nested_submsg TSRMLS_CC); + use_nested_submsg, false TSRMLS_CC); desc->intern->klass = desc->klass; if (desc->klass == NULL) { @@ -979,9 +989,13 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, const upb_enumdef *enumdef = upb_filedef_enum(file, i); CREATE_HASHTABLE_VALUE(desc, desc_php, EnumDescriptor, enum_descriptor_type); desc->enumdef = enumdef; + desc->intern = SYS_MALLOC(EnumDescriptorInternal); + desc->intern->enumdef = enumdef; + add_def_obj(desc->enumdef, desc_php); + add_enumdef_enumdesc(desc->enumdef, desc->intern); desc->klass = register_class(file, upb_enumdef_fullname(enumdef), desc_php, - use_nested_submsg TSRMLS_CC); + use_nested_submsg, true TSRMLS_CC); if (desc->klass == NULL) { return; diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 40e3a18450d2..fe30ea5307c9 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -1556,7 +1556,7 @@ PHP_METHOD(Any, unpack) { zval_dtor(&value_member); PHP_PROTO_FAKE_SCOPE_END; - merge_from_string(Z_STRVAL_P(value), Z_STRLEN_P(value), desc, msg); + merge_from_string(Z_STRVAL_P(value), Z_STRLEN_P(value), desc->intern, msg); } PHP_METHOD(Any, pack) { diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index c6a4ed6de437..3f3d27172b04 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -49,9 +49,12 @@ static HashTable* ce_to_php_obj_map; // wrapper Descriptor/EnumDescriptor instances. static HashTable* proto_to_php_obj_map; -upb_inttable upb_def_to_php_obj_map_persistent; -upb_inttable ce_to_php_obj_map_persistent; -upb_inttable proto_to_php_obj_map_persistent; +upb_inttable upb_def_to_desc_map_persistent; +upb_inttable upb_def_to_enumdesc_map_persistent; +upb_inttable ce_to_desc_map_persistent; +upb_inttable ce_to_enumdesc_map_persistent; +upb_strtable proto_to_desc_map_persistent; +upb_strtable proto_to_enumdesc_map_persistent; upb_strtable reserved_names; // ----------------------------------------------------------------------------- @@ -106,8 +109,13 @@ void add_def_obj(const void* def, PHP_PROTO_HASHTABLE_VALUE value) { add_to_table(upb_def_to_php_obj_map, def, value); } +PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def) { + return (PHP_PROTO_HASHTABLE_VALUE)get_from_table(upb_def_to_php_obj_map, def); +} + void add_msgdef_desc(const upb_msgdef* m, DescriptorInternal* desc) { - upb_inttable_insertptr(&upb_def_to_php_obj_map_persistent, m, upb_value_ptr(desc)); + upb_inttable_insertptr(&upb_def_to_desc_map_persistent, + m, upb_value_ptr(desc)); } DescriptorInternal* get_msgdef_desc(const upb_msgdef* m) { @@ -115,15 +123,28 @@ DescriptorInternal* get_msgdef_desc(const upb_msgdef* m) { #ifndef NDEBUG v.ctype = UPB_CTYPE_PTR; #endif - if (!upb_inttable_lookupptr(&upb_def_to_php_obj_map_persistent, m, &v)) { + if (!upb_inttable_lookupptr(&upb_def_to_desc_map_persistent, m, &v)) { return NULL; } else { return upb_value_getptr(v); } } -PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def) { - return (PHP_PROTO_HASHTABLE_VALUE)get_from_table(upb_def_to_php_obj_map, def); +void add_enumdef_enumdesc(const upb_enumdef* e, EnumDescriptorInternal* desc) { + upb_inttable_insertptr(&upb_def_to_enumdesc_map_persistent, + e, upb_value_ptr(desc)); +} + +EnumDescriptorInternal* get_enumdef_enumdesc(const upb_enumdef* e) { + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_PTR; +#endif + if (!upb_inttable_lookupptr(&upb_def_to_enumdesc_map_persistent, e, &v)) { + return NULL; + } else { + return upb_value_getptr(v); + } } void add_ce_obj(const void* ce, PHP_PROTO_HASHTABLE_VALUE value) { @@ -139,6 +160,40 @@ PHP_PROTO_HASHTABLE_VALUE get_ce_obj(const void* ce) { return (PHP_PROTO_HASHTABLE_VALUE)get_from_table(ce_to_php_obj_map, ce); } +void add_ce_desc(const zend_class_entry* ce, DescriptorInternal* desc) { + upb_inttable_insertptr(&ce_to_desc_map_persistent, + ce, upb_value_ptr(desc)); +} + +DescriptorInternal* get_ce_desc(const zend_class_entry* ce) { + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_PTR; +#endif + if (!upb_inttable_lookupptr(&upb_def_to_desc_map_persistent, ce, &v)) { + return NULL; + } else { + return upb_value_getptr(v); + } +} + +void add_ce_enumdesc(const zend_class_entry* ce, EnumDescriptorInternal* desc) { + upb_inttable_insertptr(&ce_to_enumdesc_map_persistent, + ce, upb_value_ptr(desc)); +} + +EnumDescriptorInternal* get_ce_enumdesc(const zend_class_entry* ce) { + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_PTR; +#endif + if (!upb_inttable_lookupptr(&upb_def_to_enumdesc_map_persistent, ce, &v)) { + return NULL; + } else { + return upb_value_getptr(v); + } +} + bool class_added(const void* ce) { return exist_in_table(ce_to_php_obj_map, ce); } @@ -157,6 +212,42 @@ PHP_PROTO_HASHTABLE_VALUE get_proto_obj(const char* proto) { proto, strlen(proto)); } +void add_proto_desc(const char* proto, DescriptorInternal* desc) { + upb_strtable_insert2(&proto_to_desc_map_persistent, proto, + strlen(proto), upb_value_ptr(desc)); +} + +DescriptorInternal* get_proto_desc(const char* proto) { + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_PTR; +#endif + if (!upb_strtable_lookupptr(&proto_to_desc_map_persistent, + proto, strlen(proto), &v)) { + return NULL; + } else { + return upb_value_getptr(v); + } +} + +void add_proto_enumdesc(const char* proto, EnumDescriptorInternal* desc) { + upb_strtable_insert2(&proto_to_enumdesc_map_persistent, proto, + strlen(proto), upb_value_ptr(desc)); +} + +EnumDescriptorInternal* get_proto_enumdesc(const char* proto) { + upb_value v; +#ifndef NDEBUG + v.ctype = UPB_CTYPE_PTR; +#endif + if (!upb_strtable_lookupptr(&proto_to_enumdesc_map_persistent, + proto, strlen(proto), &v)) { + return NULL; + } else { + return upb_value_getptr(v); + } +} + // ----------------------------------------------------------------------------- // Well Known Types. // ----------------------------------------------------------------------------- @@ -349,9 +440,12 @@ static void reserved_names_init() { } static PHP_MINIT_FUNCTION(protobuf) { - upb_inttable_init(&upb_def_to_php_obj_map_persistent, UPB_CTYPE_PTR); - upb_inttable_init(&ce_to_php_obj_map_persistent, UPB_CTYPE_PTR); - upb_inttable_init(&proto_to_php_obj_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&upb_def_to_desc_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&upb_def_to_enumdesc_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&ce_to_desc_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&ce_to_enumdesc_map_persistent, UPB_CTYPE_PTR); + upb_strtable_init(&proto_to_desc_map_persistent, UPB_CTYPE_PTR); + upb_strtable_init(&proto_to_enumdesc_map_persistent, UPB_CTYPE_PTR); upb_strtable_init(&reserved_names, UPB_CTYPE_UINT64); reserved_names_init(); @@ -429,12 +523,30 @@ static void cleanup_inttable(upb_inttable* t) { } } -static PHP_MSHUTDOWN_FUNCTION(protobuf) { - cleanup_inttable(&upb_def_to_php_obj_map_persistent); +static void cleanup_strtable(upb_strtable* t) { + upb_strtable_iter i; + upb_value v; + void* ptr; + for(upb_strtable_begin(&i, t); + !upb_strtable_done(&i); + upb_strtable_next(&i)) { + v = upb_strtable_iter_value(&i); + ptr = upb_value_getptr(v); + SYS_FREE(ptr); + } +} - upb_inttable_uninit(&upb_def_to_php_obj_map_persistent); - upb_inttable_uninit(&ce_to_php_obj_map_persistent); - upb_inttable_uninit(&proto_to_php_obj_map_persistent); +static PHP_MSHUTDOWN_FUNCTION(protobuf) { + // Only needs to clean one map out of three (def=>desc, ce=>desc, proto=>desc) + cleanup_inttable(&upb_def_to_desc_map_persistent); + cleanup_inttable(&upb_def_to_enumdesc_map_persistent); + + upb_inttable_uninit(&upb_def_to_desc_map_persistent); + upb_inttable_uninit(&upb_def_to_enumdesc_map_persistent); + upb_inttable_uninit(&ce_to_desc_map_persistent); + upb_inttable_uninit(&ce_to_enumdesc_map_persistent); + upb_strtable_uninit(&proto_to_desc_map_persistent); + upb_strtable_uninit(&proto_to_enumdesc_map_persistent); upb_strtable_uninit(&reserved_names); PEFREE(message_handlers); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index cf9444cc1dc4..c7999a2cb56a 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -596,6 +596,7 @@ struct DoubleValue; struct Duration; struct Enum; struct EnumDescriptor; +struct EnumDescriptorInternal; struct EnumValue; struct EnumValueDescriptor; struct Field; @@ -641,6 +642,7 @@ typedef struct DescriptorPool DescriptorPool; typedef struct DoubleValue DoubleValue; typedef struct Duration Duration; typedef struct EnumDescriptor EnumDescriptor; +typedef struct EnumDescriptorInternal EnumDescriptorInternal; typedef struct Enum Enum; typedef struct EnumValueDescriptor EnumValueDescriptor; typedef struct EnumValue EnumValue; @@ -755,17 +757,27 @@ void add_def_obj(const void* def, PHP_PROTO_HASHTABLE_VALUE value); PHP_PROTO_HASHTABLE_VALUE get_def_obj(const void* def); void add_msgdef_desc(const upb_msgdef* m, DescriptorInternal* desc); DescriptorInternal* get_msgdef_desc(const upb_msgdef* m); +void add_enumdef_enumdesc(const upb_enumdef* e, EnumDescriptorInternal* desc); +EnumDescriptorInternal* get_enumdef_enumdesc(const upb_enumdef* e); // Global map from PHP class entries to wrapper Descriptor/EnumDescriptor // instances. void add_ce_obj(const void* ce, PHP_PROTO_HASHTABLE_VALUE value); PHP_PROTO_HASHTABLE_VALUE get_ce_obj(const void* ce); bool class_added(const void* ce); +void add_ce_desc(const zend_class_entry* ce, DescriptorInternal* desc); +DescriptorInternal* get_ce_desc(const zend_class_entry* ce); +void add_ce_enumdesc(const zend_class_entry* ce, EnumDescriptorInternal* desc); +EnumDescriptorInternal* get_ce_enumdesc(const zend_class_entry* ce); // Global map from message/enum's proto fully-qualified name to corresponding // wrapper Descriptor/EnumDescriptor instances. void add_proto_obj(const char* proto, PHP_PROTO_HASHTABLE_VALUE value); PHP_PROTO_HASHTABLE_VALUE get_proto_obj(const char* proto); +void add_proto_desc(const char* proto, DescriptorInternal* desc); +DescriptorInternal* get_proto_desc(const char* proto); +void add_proto_enumdesc(const char* proto, EnumDescriptorInternal* desc); +EnumDescriptorInternal* get_proto_enumdesc(const char* proto); extern zend_class_entry* map_field_type; extern zend_class_entry* repeated_field_type; @@ -855,9 +867,15 @@ PHP_METHOD(FieldDescriptor, getMessageType); extern zend_class_entry* field_descriptor_type; +struct EnumDescriptorInternal { + const upb_enumdef* enumdef; + zend_class_entry* klass; // begins as NULL +}; + PHP_PROTO_WRAP_OBJECT_START(EnumDescriptor) const upb_enumdef* enumdef; zend_class_entry* klass; // begins as NULL + EnumDescriptorInternal* intern; PHP_PROTO_WRAP_OBJECT_END PHP_METHOD(EnumDescriptor, getValue); diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 4a8543f7b948..58216ad8c8fc 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -550,8 +550,7 @@ const upb_fielddef* map_entry_value(const upb_msgdef* msgdef) { const zend_class_entry* field_type_class( const upb_fielddef* field PHP_PROTO_TSRMLS_DC) { if (upb_fielddef_type(field) == UPB_TYPE_MESSAGE) { - Descriptor* desc = UNBOX_HASHTABLE_VALUE( - Descriptor, get_def_obj(upb_fielddef_msgsubdef(field))); + DescriptorInternal* desc = get_msgdef_desc(upb_fielddef_msgsubdef(field)); return desc->klass; } else if (upb_fielddef_type(field) == UPB_TYPE_ENUM) { EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE( @@ -600,7 +599,7 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { layout->empty_template = NULL; TSRMLS_FETCH(); - Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msgdef)); + DescriptorInternal* desc = get_msgdef_desc(msgdef); layout->fields = ALLOC_N(MessageField, nfields); for (upb_msg_field_begin(&it, msgdef); !upb_msg_field_done(&it); @@ -822,9 +821,7 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header, const upb_msgdef* submsgdef = upb_fielddef_msgsubdef(field); const upb_fielddef* value_field = upb_msgdef_itof(submsgdef, 1); MessageHeader* submsg; - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE( - Descriptor, get_def_obj((void*)submsgdef)); + DescriptorInternal* subdesc = get_msgdef_desc(submsgdef); zend_class_entry* subklass = subdesc->klass; #if PHP_MAJOR_VERSION < 7 zval* val = NULL; @@ -879,8 +876,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, if (upb_fielddef_descriptortype(valuefield) == UPB_DESCRIPTOR_TYPE_MESSAGE) { const upb_msgdef* submsg = upb_fielddef_msgsubdef(valuefield); - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(submsg)); + DescriptorInternal* subdesc = get_msgdef_desc(submsg); subce = subdesc->klass; } check_map_field(subce, upb_fielddef_descriptortype(keyfield), @@ -889,8 +885,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, } else { if (upb_fielddef_type(field) == UPB_TYPE_MESSAGE) { const upb_msgdef* submsg = upb_fielddef_msgsubdef(field); - Descriptor* subdesc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(submsg)); + DescriptorInternal* subdesc = get_msgdef_desc(submsg); subce = subdesc->klass; } @@ -912,7 +907,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, zend_class_entry *ce = NULL; if (type == UPB_TYPE_MESSAGE) { const upb_msgdef* msg = upb_fielddef_msgsubdef(field); - Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg)); + DescriptorInternal* desc = get_msgdef_desc(msg); ce = desc->klass; } CACHED_VALUE* cache = find_zval_property(header, field); @@ -950,7 +945,7 @@ static void native_slot_merge( break; case UPB_TYPE_MESSAGE: { const upb_msgdef* msg = upb_fielddef_msgsubdef(field); - Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg)); + DescriptorInternal* desc = get_msgdef_desc(msg); ce = desc->klass; if (native_slot_is_default(type, to_memory)) { #if PHP_MAJOR_VERSION < 7 @@ -996,7 +991,7 @@ static void native_slot_merge_by_array(const upb_fielddef* field, const void* fr } case UPB_TYPE_MESSAGE: { const upb_msgdef* msg = upb_fielddef_msgsubdef(field); - Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg)); + DescriptorInternal* desc = get_msgdef_desc(upb_fielddef_msgsubdef(field)); zend_class_entry* ce = desc->klass; #if PHP_MAJOR_VERSION < 7 MAKE_STD_ZVAL(DEREF(to_memory, zval*)); diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index da5f3f3ac14b..da109f4efcec 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -12,7 +12,8 @@ php -i | grep "Configuration" # 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 encode_decode_test.php +gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php well_known_test.php # # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # From 3d2e774b3db1896362d1cf50a81c94a99b276226 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 21:01:38 +0000 Subject: [PATCH 05/16] Replace get_ce_obj --- php/ext/google/protobuf/def.c | 1 + php/ext/google/protobuf/encode_decode.c | 28 +++++++++++-------------- php/ext/google/protobuf/message.c | 13 +++++------- php/ext/google/protobuf/protobuf.c | 4 ++-- php/ext/google/protobuf/protobuf.h | 3 ++- 5 files changed, 22 insertions(+), 27 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 9c74a30daffa..a87305f23e5c 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -996,6 +996,7 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, add_enumdef_enumdesc(desc->enumdef, desc->intern); desc->klass = register_class(file, upb_enumdef_fullname(enumdef), desc_php, use_nested_submsg, true TSRMLS_CC); + desc->intern->klass = desc->klass; if (desc->klass == NULL) { return; diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index f9128b125437..14808eb1ae15 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -2059,14 +2059,13 @@ static const upb_handlers* msgdef_json_serialize_handlers( // ----------------------------------------------------------------------------- void serialize_to_string(zval* val, zval* return_value TSRMLS_DC) { - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(Z_OBJCE_P(val))); + DescriptorInternal* desc = get_ce_desc(Z_OBJCE_P(val)); stringsink sink; stringsink_init(&sink); { - const upb_handlers* serialize_handlers = msgdef_pb_serialize_handlers(desc->intern); + const upb_handlers* serialize_handlers = msgdef_pb_serialize_handlers(desc); stackenv se; upb_pb_encoder* encoder; @@ -2074,7 +2073,7 @@ void serialize_to_string(zval* val, zval* return_value TSRMLS_DC) { stackenv_init(&se, "Error occurred during encoding: %s"); encoder = upb_pb_encoder_create(se.arena, serialize_handlers, sink.sink); - putmsg(val, desc->intern, upb_pb_encoder_input(encoder), 0, false TSRMLS_CC); + putmsg(val, desc, upb_pb_encoder_input(encoder), 0, false TSRMLS_CC); PHP_PROTO_RETVAL_STRINGL(sink.ptr, sink.len, 1); @@ -2120,8 +2119,7 @@ void merge_from_string(const char* data, int data_len, DescriptorInternal* desc, } PHP_METHOD(Message, mergeFromString) { - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(Z_OBJCE_P(getThis()))); + DescriptorInternal* desc = get_ce_desc(Z_OBJCE_P(getThis())); MessageHeader* msg = UNBOX(MessageHeader, getThis()); char *data = NULL; @@ -2132,12 +2130,11 @@ PHP_METHOD(Message, mergeFromString) { return; } - merge_from_string(data, data_len, desc->intern, msg); + merge_from_string(data, data_len, desc, msg); } PHP_METHOD(Message, serializeToJsonString) { - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(Z_OBJCE_P(getThis()))); + DescriptorInternal* desc = get_ce_desc(Z_OBJCE_P(getThis())); zend_bool preserve_proto_fieldnames = false; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|b", @@ -2150,14 +2147,14 @@ PHP_METHOD(Message, serializeToJsonString) { { const upb_handlers* serialize_handlers = - msgdef_json_serialize_handlers(desc->intern, preserve_proto_fieldnames); + msgdef_json_serialize_handlers(desc, preserve_proto_fieldnames); upb_json_printer* printer; stackenv se; stackenv_init(&se, "Error occurred during encoding: %s"); printer = upb_json_printer_create(se.arena, serialize_handlers, sink.sink); - putmsg(getThis(), desc->intern, upb_json_printer_input(printer), 0, true TSRMLS_CC); + putmsg(getThis(), desc, upb_json_printer_input(printer), 0, true TSRMLS_CC); PHP_PROTO_RETVAL_STRINGL(sink.ptr, sink.len, 1); @@ -2167,8 +2164,7 @@ PHP_METHOD(Message, serializeToJsonString) { } PHP_METHOD(Message, mergeFromJsonString) { - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(Z_OBJCE_P(getThis()))); + DescriptorInternal* desc = get_ce_desc(Z_OBJCE_P(getThis())); MessageHeader* msg = UNBOX(MessageHeader, getThis()); char *data = NULL; @@ -2189,7 +2185,7 @@ PHP_METHOD(Message, mergeFromJsonString) { // TODO(teboring): Clear message. { - const upb_json_parsermethod* method = msgdef_jsonparsermethod(desc->intern); + const upb_json_parsermethod* method = msgdef_jsonparsermethod(desc); stackenv se; upb_sink sink; upb_json_parser* parser; @@ -2207,7 +2203,7 @@ PHP_METHOD(Message, mergeFromJsonString) { closure = msg; } - upb_sink_reset(&sink, get_fill_handlers(desc->intern), closure); + upb_sink_reset(&sink, get_fill_handlers(desc), closure); parser = upb_json_parser_create(se.arena, method, generated_pool->symtab, sink, &se.status, ignore_json_unknown); upb_bufsrc_putbuf(data, data_len, upb_json_parser_input(parser)); @@ -2231,7 +2227,7 @@ static void discard_unknown_fields(MessageHeader* msg) { } // Recursively discard unknown fields of submessages. - Descriptor* desc = msg->descriptor; + DescriptorInternal* desc = msg->descriptor; TSRMLS_FETCH(); for (upb_msg_field_begin(&it, desc->msgdef); !upb_msg_field_done(&it); diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index fe30ea5307c9..94eca601722b 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -255,7 +255,7 @@ void* message_data(MessageHeader* msg) { 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)); + DescriptorInternal* desc = get_ce_desc(ce); intern->data = ALLOC_N(uint8_t, 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(). @@ -528,7 +528,7 @@ PHP_METHOD(Message, __construct) { PHP_METHOD(Message, clear) { MessageHeader* msg = UNBOX(MessageHeader, getThis()); - Descriptor* desc = msg->descriptor; + DescriptorInternal* desc = msg->descriptor; zend_class_entry* ce = desc->klass; zend_object_std_dtor(&msg->std TSRMLS_CC); @@ -1587,8 +1587,7 @@ PHP_METHOD(Any, pack) { PHP_PROTO_FAKE_SCOPE_END; // Set type url. - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(Z_OBJCE_P(val))); + DescriptorInternal* desc = get_ce_desc(Z_OBJCE_P(val)); const char* fully_qualified_name = upb_msgdef_fullname(desc->msgdef); size_t type_url_len = strlen(TYPE_URL_PREFIX) + strlen(fully_qualified_name) + 1; @@ -1615,14 +1614,12 @@ PHP_METHOD(Any, is) { return; } - PHP_PROTO_HASHTABLE_VALUE desc_php = get_ce_obj(klass); - if (desc_php == NULL) { + DescriptorInternal* desc = get_ce_desc(klass); + if (desc == NULL) { RETURN_BOOL(false); } // Create corresponded type url. - Descriptor* desc = - UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(klass)); const char* fully_qualified_name = upb_msgdef_fullname(desc->msgdef); size_t type_url_len = strlen(TYPE_URL_PREFIX) + strlen(fully_qualified_name) + 1; diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 3f3d27172b04..54a510f0e7ab 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -170,7 +170,7 @@ DescriptorInternal* get_ce_desc(const zend_class_entry* ce) { #ifndef NDEBUG v.ctype = UPB_CTYPE_PTR; #endif - if (!upb_inttable_lookupptr(&upb_def_to_desc_map_persistent, ce, &v)) { + if (!upb_inttable_lookupptr(&ce_to_desc_map_persistent, ce, &v)) { return NULL; } else { return upb_value_getptr(v); @@ -187,7 +187,7 @@ EnumDescriptorInternal* get_ce_enumdesc(const zend_class_entry* ce) { #ifndef NDEBUG v.ctype = UPB_CTYPE_PTR; #endif - if (!upb_inttable_lookupptr(&upb_def_to_enumdesc_map_persistent, ce, &v)) { + if (!upb_inttable_lookupptr(&ce_to_enumdesc_map_persistent, ce, &v)) { return NULL; } else { return upb_value_getptr(v); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index c7999a2cb56a..da651dba5668 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -981,7 +981,8 @@ struct MessageLayout { PHP_PROTO_WRAP_OBJECT_START(MessageHeader) void* data; // Point to the real message data. // Place needs to be consistent with map_parse_frame_data_t. - Descriptor* descriptor; // Kept alive by self.class.descriptor reference. + DescriptorInternal* descriptor; // Kept alive by self.class.descriptor + // reference. PHP_PROTO_WRAP_OBJECT_END MessageLayout* create_layout(const upb_msgdef* msgdef); From bd9918d429a39cf74de3f7273c66666e4acafa81 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 21:24:01 +0000 Subject: [PATCH 06/16] Remove get_proto_obj --- php/ext/google/protobuf/def.c | 1 - php/ext/google/protobuf/message.c | 7 +++-- php/ext/google/protobuf/protobuf.c | 42 +++++++----------------------- php/ext/google/protobuf/protobuf.h | 2 -- 4 files changed, 13 insertions(+), 39 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index a87305f23e5c..6104fdc93cac 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -867,7 +867,6 @@ static zend_class_entry *register_class(const upb_filedef *file, } ret = PHP_PROTO_CE_UNREF(pce); add_ce_obj(ret, desc_php); - add_proto_obj(fullname, desc_php); if (is_enum) { EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE(EnumDescriptor, desc_php); add_ce_enumdesc(ret, desc->intern); diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 94eca601722b..ff1c3c4f2c53 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -1534,14 +1534,13 @@ PHP_METHOD(Any, unpack) { } const char* fully_qualified_name = type_url + url_prefix_len; - PHP_PROTO_HASHTABLE_VALUE desc_php = get_proto_obj(fully_qualified_name); - if (desc_php == NULL) { + DescriptorInternal* desc = get_proto_desc(fully_qualified_name); + if (desc == NULL) { zend_throw_exception( NULL, "Specified message in any hasn't been added to descriptor pool", 0 TSRMLS_CC); return; } - Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); zend_class_entry* klass = desc->klass; ZVAL_OBJ(return_value, klass->create_object(klass TSRMLS_CC)); MessageHeader* msg = UNBOX(MessageHeader, return_value); @@ -1556,7 +1555,7 @@ PHP_METHOD(Any, unpack) { zval_dtor(&value_member); PHP_PROTO_FAKE_SCOPE_END; - merge_from_string(Z_STRVAL_P(value), Z_STRLEN_P(value), desc->intern, msg); + merge_from_string(Z_STRVAL_P(value), Z_STRLEN_P(value), desc, msg); } PHP_METHOD(Any, pack) { diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 54a510f0e7ab..55dcceef9c4b 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -42,19 +42,18 @@ static PHP_MSHUTDOWN_FUNCTION(protobuf); // Global map from upb {msg,enum}defs to wrapper Descriptor/EnumDescriptor // instances. static HashTable* upb_def_to_php_obj_map; +static upb_inttable upb_def_to_desc_map_persistent; +static upb_inttable upb_def_to_enumdesc_map_persistent; // Global map from message/enum's php class entry to corresponding wrapper // Descriptor/EnumDescriptor instances. static HashTable* ce_to_php_obj_map; +static upb_inttable ce_to_desc_map_persistent; +static upb_inttable ce_to_enumdesc_map_persistent; // Global map from message/enum's proto fully-qualified name to corresponding // wrapper Descriptor/EnumDescriptor instances. -static HashTable* proto_to_php_obj_map; - -upb_inttable upb_def_to_desc_map_persistent; -upb_inttable upb_def_to_enumdesc_map_persistent; -upb_inttable ce_to_desc_map_persistent; -upb_inttable ce_to_enumdesc_map_persistent; -upb_strtable proto_to_desc_map_persistent; -upb_strtable proto_to_enumdesc_map_persistent; +static upb_strtable proto_to_desc_map_persistent; +static upb_strtable proto_to_enumdesc_map_persistent; + upb_strtable reserved_names; // ----------------------------------------------------------------------------- @@ -198,23 +197,9 @@ bool class_added(const void* ce) { return exist_in_table(ce_to_php_obj_map, ce); } -void add_proto_obj(const char* proto, PHP_PROTO_HASHTABLE_VALUE value) { -#if PHP_MAJOR_VERSION < 7 - Z_ADDREF_P(value); -#else - GC_ADDREF(value); -#endif - add_to_strtable(proto_to_php_obj_map, proto, strlen(proto), value); -} - -PHP_PROTO_HASHTABLE_VALUE get_proto_obj(const char* proto) { - return (PHP_PROTO_HASHTABLE_VALUE)get_from_strtable(proto_to_php_obj_map, - proto, strlen(proto)); -} - void add_proto_desc(const char* proto, DescriptorInternal* desc) { - upb_strtable_insert2(&proto_to_desc_map_persistent, proto, - strlen(proto), upb_value_ptr(desc)); + upb_strtable_insert(&proto_to_desc_map_persistent, proto, + upb_value_ptr(desc)); } DescriptorInternal* get_proto_desc(const char* proto) { @@ -222,8 +207,7 @@ DescriptorInternal* get_proto_desc(const char* proto) { #ifndef NDEBUG v.ctype = UPB_CTYPE_PTR; #endif - if (!upb_strtable_lookupptr(&proto_to_desc_map_persistent, - proto, strlen(proto), &v)) { + if (!upb_strtable_lookup(&proto_to_desc_map_persistent, proto, &v)) { return NULL; } else { return upb_value_getptr(v); @@ -360,9 +344,6 @@ static PHP_RINIT_FUNCTION(protobuf) { ALLOC_HASHTABLE(ce_to_php_obj_map); zend_hash_init(ce_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); - ALLOC_HASHTABLE(proto_to_php_obj_map); - zend_hash_init(proto_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); - generated_pool = NULL; generated_pool_php = NULL; internal_generated_pool_php = NULL; @@ -388,9 +369,6 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { zend_hash_destroy(ce_to_php_obj_map); FREE_HASHTABLE(ce_to_php_obj_map); - zend_hash_destroy(proto_to_php_obj_map); - FREE_HASHTABLE(proto_to_php_obj_map); - #if PHP_MAJOR_VERSION < 7 if (generated_pool_php != NULL) { zval_dtor(generated_pool_php); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index da651dba5668..0af0b82d9752 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -772,8 +772,6 @@ EnumDescriptorInternal* get_ce_enumdesc(const zend_class_entry* ce); // Global map from message/enum's proto fully-qualified name to corresponding // wrapper Descriptor/EnumDescriptor instances. -void add_proto_obj(const char* proto, PHP_PROTO_HASHTABLE_VALUE value); -PHP_PROTO_HASHTABLE_VALUE get_proto_obj(const char* proto); void add_proto_desc(const char* proto, DescriptorInternal* desc); DescriptorInternal* get_proto_desc(const char* proto); void add_proto_enumdesc(const char* proto, EnumDescriptorInternal* desc); From 99803ca201342e82e40c3b9eda82b5042ebae9dc Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 22:24:03 +0000 Subject: [PATCH 07/16] Remove obsolete fields from Descriptor and EnumDescriptor --- php/ext/google/protobuf/def.c | 64 +++++++++++++++--------------- php/ext/google/protobuf/message.c | 9 ++--- php/ext/google/protobuf/protobuf.c | 33 ++++++++------- php/ext/google/protobuf/protobuf.h | 7 +--- php/ext/google/protobuf/storage.c | 16 ++++---- 5 files changed, 64 insertions(+), 65 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 6104fdc93cac..7212fa297cde 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -148,19 +148,15 @@ static zend_function_entry descriptor_methods[] = { DEFINE_CLASS(Descriptor, descriptor, "Google\\Protobuf\\Descriptor"); static void descriptor_free_c(Descriptor *self TSRMLS_DC) { - if (self->layout) { - free_layout(self->layout); - } } static void descriptor_init_c_instance(Descriptor *desc TSRMLS_DC) { - desc->msgdef = NULL; - desc->layout = NULL; - desc->klass = NULL; + desc->intern = NULL; } PHP_METHOD(Descriptor, getClass) { - Descriptor *intern = UNBOX(Descriptor, getThis()); + Descriptor* desc = UNBOX(Descriptor, getThis()); + DescriptorInternal* intern = desc->intern; #if PHP_MAJOR_VERSION < 7 const char* classname = intern->klass->name; #else @@ -170,7 +166,8 @@ PHP_METHOD(Descriptor, getClass) { } PHP_METHOD(Descriptor, getFullName) { - Descriptor *intern = UNBOX(Descriptor, getThis()); + Descriptor* desc = UNBOX(Descriptor, getThis()); + DescriptorInternal* intern = desc->intern; const char* fullname = upb_msgdef_fullname(intern->msgdef); PHP_PROTO_RETVAL_STRINGL(fullname, strlen(fullname), 1); } @@ -183,7 +180,8 @@ PHP_METHOD(Descriptor, getField) { return; } - Descriptor *intern = UNBOX(Descriptor, getThis()); + Descriptor* desc = UNBOX(Descriptor, getThis()); + DescriptorInternal* intern = desc->intern; int field_num = upb_msgdef_numfields(intern->msgdef); if (index < 0 || index >= field_num) { zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); @@ -224,7 +222,8 @@ PHP_METHOD(Descriptor, getField) { } PHP_METHOD(Descriptor, getFieldCount) { - Descriptor *intern = UNBOX(Descriptor, getThis()); + Descriptor* desc = UNBOX(Descriptor, getThis()); + DescriptorInternal* intern = desc->intern; RETURN_LONG(upb_msgdef_numfields(intern->msgdef)); } @@ -236,7 +235,8 @@ PHP_METHOD(Descriptor, getOneofDecl) { return; } - Descriptor *intern = UNBOX(Descriptor, getThis()); + Descriptor* desc = UNBOX(Descriptor, getThis()); + DescriptorInternal* intern = desc->intern; int field_num = upb_msgdef_numoneofs(intern->msgdef); if (index < 0 || index >= field_num) { zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); @@ -257,7 +257,8 @@ PHP_METHOD(Descriptor, getOneofDecl) { } PHP_METHOD(Descriptor, getOneofDeclCount) { - Descriptor *intern = UNBOX(Descriptor, getThis()); + Descriptor* desc = UNBOX(Descriptor, getThis()); + DescriptorInternal* intern = desc->intern; RETURN_LONG(upb_msgdef_numoneofs(intern->msgdef)); } @@ -278,8 +279,7 @@ static void enum_descriptor_free_c(EnumDescriptor *self TSRMLS_DC) { } static void enum_descriptor_init_c_instance(EnumDescriptor *self TSRMLS_DC) { - self->enumdef = NULL; - self->klass = NULL; + self->intern = NULL; } PHP_METHOD(EnumDescriptor, getValue) { @@ -290,7 +290,8 @@ PHP_METHOD(EnumDescriptor, getValue) { return; } - EnumDescriptor *intern = UNBOX(EnumDescriptor, getThis()); + EnumDescriptor *desc = UNBOX(EnumDescriptor, getThis()); + EnumDescriptorInternal *intern = desc->intern; int field_num = upb_enumdef_numvals(intern->enumdef); if (index < 0 || index >= field_num) { zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); @@ -312,7 +313,8 @@ PHP_METHOD(EnumDescriptor, getValue) { } PHP_METHOD(EnumDescriptor, getValueCount) { - EnumDescriptor *intern = UNBOX(EnumDescriptor, getThis()); + EnumDescriptor *desc = UNBOX(EnumDescriptor, getThis()); + EnumDescriptorInternal *intern = desc->intern; RETURN_LONG(upb_enumdef_numvals(intern->enumdef)); } @@ -958,14 +960,14 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, for (i = 0; i < upb_filedef_msgcount(file); i++) { const upb_msgdef *msgdef = upb_filedef_msg(file, i); CREATE_HASHTABLE_VALUE(desc, desc_php, Descriptor, descriptor_type); - desc->msgdef = msgdef; - desc->pool = pool; desc->intern = SYS_MALLOC(DescriptorInternal); desc->intern->msgdef = msgdef; desc->intern->pool = pool; + desc->intern->layout = NULL; + desc->intern->klass = NULL; - add_def_obj(desc->msgdef, desc_php); - add_msgdef_desc(desc->msgdef, desc->intern); + add_def_obj(desc->intern->msgdef, desc_php); + add_msgdef_desc(desc->intern->msgdef, desc->intern); // Unlike other messages, MapEntry is shared by all map fields and doesn't // have generated PHP class. @@ -973,11 +975,11 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, continue; } - desc->klass = register_class(file, upb_msgdef_fullname(msgdef), desc_php, - use_nested_submsg, false TSRMLS_CC); - desc->intern->klass = desc->klass; + desc->intern->klass = + register_class(file, upb_msgdef_fullname(msgdef), desc_php, + use_nested_submsg, false TSRMLS_CC); - if (desc->klass == NULL) { + if (desc->intern->klass == NULL) { return; } @@ -987,17 +989,17 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, for (i = 0; i < upb_filedef_enumcount(file); i++) { const upb_enumdef *enumdef = upb_filedef_enum(file, i); CREATE_HASHTABLE_VALUE(desc, desc_php, EnumDescriptor, enum_descriptor_type); - desc->enumdef = enumdef; desc->intern = SYS_MALLOC(EnumDescriptorInternal); desc->intern->enumdef = enumdef; + desc->intern->klass = NULL; - add_def_obj(desc->enumdef, desc_php); - add_enumdef_enumdesc(desc->enumdef, desc->intern); - desc->klass = register_class(file, upb_enumdef_fullname(enumdef), desc_php, - use_nested_submsg, true TSRMLS_CC); - desc->intern->klass = desc->klass; + add_def_obj(desc->intern->enumdef, desc_php); + add_enumdef_enumdesc(desc->intern->enumdef, desc->intern); + desc->intern->klass = + register_class(file, upb_enumdef_fullname(enumdef), desc_php, + use_nested_submsg, true TSRMLS_CC); - if (desc->klass == NULL) { + if (desc->intern->klass == NULL) { return; } } diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index ff1c3c4f2c53..e80ec4ec37bb 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -279,15 +279,14 @@ void build_class_from_descriptor( Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, php_descriptor); // Map entries don't have existing php class. - if (upb_msgdef_mapentry(desc->msgdef)) { + if (upb_msgdef_mapentry(desc->intern->msgdef)) { return; } - zend_class_entry* registered_ce = desc->klass; + zend_class_entry* registered_ce = desc->intern->klass; - if (desc->layout == NULL) { - MessageLayout* layout = create_layout(desc->msgdef); - desc->layout = layout; + if (desc->intern->layout == NULL) { + MessageLayout* layout = create_layout(desc->intern->msgdef); desc->intern->layout = layout; } diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 55dcceef9c4b..8e3b9db6b143 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -488,36 +488,39 @@ static PHP_MINIT_FUNCTION(protobuf) { return 0; } -static void cleanup_inttable(upb_inttable* t) { +static void cleanup_desc_table(upb_inttable* t) { upb_inttable_iter i; upb_value v; - void* ptr; + DescriptorInternal* desc; for(upb_inttable_begin(&i, t); !upb_inttable_done(&i); upb_inttable_next(&i)) { v = upb_inttable_iter_value(&i); - ptr = upb_value_getptr(v); - SYS_FREE(ptr); + desc = upb_value_getptr(v); + if (desc->layout) { + free_layout(desc->layout); + } + SYS_FREE(desc); } } -static void cleanup_strtable(upb_strtable* t) { - upb_strtable_iter i; +static void cleanup_enumdesc_table(upb_inttable* t) { + upb_inttable_iter i; upb_value v; - void* ptr; - for(upb_strtable_begin(&i, t); - !upb_strtable_done(&i); - upb_strtable_next(&i)) { - v = upb_strtable_iter_value(&i); - ptr = upb_value_getptr(v); - SYS_FREE(ptr); + EnumDescriptorInternal* desc; + for(upb_inttable_begin(&i, t); + !upb_inttable_done(&i); + upb_inttable_next(&i)) { + v = upb_inttable_iter_value(&i); + desc = upb_value_getptr(v); + SYS_FREE(desc); } } static PHP_MSHUTDOWN_FUNCTION(protobuf) { // Only needs to clean one map out of three (def=>desc, ce=>desc, proto=>desc) - cleanup_inttable(&upb_def_to_desc_map_persistent); - cleanup_inttable(&upb_def_to_enumdesc_map_persistent); + cleanup_desc_table(&upb_def_to_desc_map_persistent); + cleanup_enumdesc_table(&upb_def_to_enumdesc_map_persistent); upb_inttable_uninit(&upb_def_to_desc_map_persistent); upb_inttable_uninit(&upb_def_to_enumdesc_map_persistent); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 0af0b82d9752..663f0d4d8b1c 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -833,10 +833,6 @@ struct DescriptorInternal { }; PHP_PROTO_WRAP_OBJECT_START(Descriptor) - InternalDescriptorPool* pool; - const upb_msgdef* msgdef; - MessageLayout* layout; - zend_class_entry* klass; // begins as NULL DescriptorInternal* intern; PHP_PROTO_WRAP_OBJECT_END @@ -871,8 +867,6 @@ struct EnumDescriptorInternal { }; PHP_PROTO_WRAP_OBJECT_START(EnumDescriptor) - const upb_enumdef* enumdef; - zend_class_entry* klass; // begins as NULL EnumDescriptorInternal* intern; PHP_PROTO_WRAP_OBJECT_END @@ -1529,6 +1523,7 @@ size_t stringsink_string(void *_sink, const void *hd, const char *ptr, // Memory management #define SYS_MALLOC(class_name) (class_name*) malloc(sizeof(class_name)) +#define SYS_MALLOC_N(class_name, n) (class_name*) malloc(sizeof(class_name) * n) #define SYS_FREE(ptr) free(ptr) #define ALLOC(class_name) (class_name*) emalloc(sizeof(class_name)) #define PEMALLOC(class_name, persistent) (class_name*) pemalloc(sizeof(class_name), persistent) diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 58216ad8c8fc..2521af5b2e18 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -553,8 +553,8 @@ const zend_class_entry* field_type_class( DescriptorInternal* desc = get_msgdef_desc(upb_fielddef_msgsubdef(field)); return desc->klass; } else if (upb_fielddef_type(field) == UPB_TYPE_ENUM) { - EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE( - EnumDescriptor, get_def_obj(upb_fielddef_enumsubdef(field))); + EnumDescriptorInternal* desc = + get_enumdef_enumdesc(upb_fielddef_enumsubdef(field)); return desc->klass; } return NULL; @@ -586,7 +586,7 @@ void* slot_memory(MessageLayout* layout, const void* storage, } MessageLayout* create_layout(const upb_msgdef* msgdef) { - MessageLayout* layout = ALLOC(MessageLayout); + MessageLayout* layout = SYS_MALLOC(MessageLayout); int nfields = upb_msgdef_numfields(msgdef); upb_msg_field_iter it; upb_msg_oneof_iter oit; @@ -600,7 +600,7 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { TSRMLS_FETCH(); DescriptorInternal* desc = get_msgdef_desc(msgdef); - layout->fields = ALLOC_N(MessageField, nfields); + layout->fields = SYS_MALLOC_N(MessageField, nfields); for (upb_msg_field_begin(&it, msgdef); !upb_msg_field_done(&it); upb_msg_field_next(&it)) { @@ -744,16 +744,16 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { layout->msgdef = msgdef; // Create the empty message template. - layout->empty_template = ALLOC_N(char, layout->size); + layout->empty_template = SYS_MALLOC_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); + SYS_FREE(layout->empty_template); + SYS_FREE(layout->fields); + SYS_FREE(layout); } void layout_init(MessageLayout* layout, void* storage, From eac0a6739d6425d046eceba44747f861445480e3 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 23:13:18 +0000 Subject: [PATCH 08/16] Add cache for descriptor php values --- php/ext/google/protobuf/def.c | 52 +++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 7212fa297cde..6a5084e27f50 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -442,13 +442,31 @@ PHP_METHOD(FieldDescriptor, getEnumType) { return; } const upb_enumdef *enumdef = upb_fielddef_enumsubdef(intern->fielddef); - PHP_PROTO_HASHTABLE_VALUE desc = get_def_obj(enumdef); + PHP_PROTO_HASHTABLE_VALUE desc_php = get_def_obj(enumdef); + + if (desc_php == NULL) { + EnumDescriptorInternal* intern = get_enumdef_enumdesc(enumdef); #if PHP_MAJOR_VERSION < 7 - RETURN_ZVAL(desc, 1, 0); + MAKE_STD_ZVAL(desc_php); + ZVAL_OBJ(desc_php, enum_descriptor_type->create_object( + enum_descriptor_type TSRMLS_CC)); + Z_DELREF_P(desc_php); #else - GC_ADDREF(desc); - RETURN_OBJ(desc); + desc_php = + enum_descriptor_type->create_object(enum_descriptor_type TSRMLS_CC); + GC_DELREF(desc_php); +#endif + EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE(EnumDescriptor, desc_php); + desc->intern = intern; + add_def_obj(enumdef, desc_php); + } + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(desc_php, 1, 0); +#else + GC_ADDREF(desc_php); + RETURN_OBJ(desc_php); #endif } @@ -461,13 +479,31 @@ PHP_METHOD(FieldDescriptor, getMessageType) { return; } const upb_msgdef *msgdef = upb_fielddef_msgsubdef(intern->fielddef); - PHP_PROTO_HASHTABLE_VALUE desc = get_def_obj(msgdef); + PHP_PROTO_HASHTABLE_VALUE desc_php = get_def_obj(msgdef); + + if (desc_php == NULL) { + DescriptorInternal* intern = get_def_desc(msgdef); #if PHP_MAJOR_VERSION < 7 - RETURN_ZVAL(desc, 1, 0); + MAKE_STD_ZVAL(desc_php); + ZVAL_OBJ(desc_php, descriptor_type->create_object( + descriptor_type TSRMLS_CC)); + Z_DELREF_P(desc_php); #else - GC_ADDREF(desc); - RETURN_OBJ(desc); + desc_php = + descriptor_type->create_object(descriptor_type TSRMLS_CC); + GC_DELREF(desc_php); +#endif + Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); + desc->intern = intern; + add_def_obj(msgdef, desc_php); + } + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(desc_php, 1, 0); +#else + GC_ADDREF(desc_php); + RETURN_OBJ(desc_php); #endif } From 87f53dd943a14f300175c2045db43f29effe7f8c Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 23:28:52 +0000 Subject: [PATCH 09/16] Add cache for descriptors --- php/ext/google/protobuf/def.c | 64 +++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 6a5084e27f50..a7cf36da3f19 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -482,7 +482,7 @@ PHP_METHOD(FieldDescriptor, getMessageType) { PHP_PROTO_HASHTABLE_VALUE desc_php = get_def_obj(msgdef); if (desc_php == NULL) { - DescriptorInternal* intern = get_def_desc(msgdef); + DescriptorInternal* intern = get_msgdef_desc(msgdef); #if PHP_MAJOR_VERSION < 7 MAKE_STD_ZVAL(desc_php); @@ -1077,22 +1077,39 @@ PHP_METHOD(DescriptorPool, getDescriptorByClassName) { RETURN_NULL(); } - PHP_PROTO_HASHTABLE_VALUE desc = get_ce_obj(PHP_PROTO_CE_UNREF(pce)); - if (desc == NULL) { - RETURN_NULL(); + PHP_PROTO_HASHTABLE_VALUE desc_php = get_ce_obj(PHP_PROTO_CE_UNREF(pce)); + if (desc_php == NULL) { + DescriptorInternal* intern = get_ce_desc(PHP_PROTO_CE_UNREF(pce)); + if (intern == NULL) { + RETURN_NULL(); + } + +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(desc_php); + ZVAL_OBJ(desc_php, descriptor_type->create_object( + descriptor_type TSRMLS_CC)); + Z_DELREF_P(desc_php); +#else + desc_php = + descriptor_type->create_object(descriptor_type TSRMLS_CC); + GC_DELREF(desc_php); +#endif + Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); + desc->intern = intern; + add_ce_obj(PHP_PROTO_CE_UNREF(pce), desc_php); } - zend_class_entry* instance_ce = HASHTABLE_VALUE_CE(desc); + zend_class_entry* instance_ce = HASHTABLE_VALUE_CE(desc_php); if (!instanceof_function(instance_ce, descriptor_type TSRMLS_CC)) { RETURN_NULL(); } #if PHP_MAJOR_VERSION < 7 - RETURN_ZVAL(desc, 1, 0); + RETURN_ZVAL(desc_php, 1, 0); #else - GC_ADDREF(desc); - RETURN_OBJ(desc); + GC_ADDREF(desc_php); + RETURN_OBJ(desc_php); #endif } @@ -1114,21 +1131,38 @@ PHP_METHOD(DescriptorPool, getEnumDescriptorByClassName) { RETURN_NULL(); } - PHP_PROTO_HASHTABLE_VALUE desc = get_ce_obj(PHP_PROTO_CE_UNREF(pce)); - if (desc == NULL) { - RETURN_NULL(); + PHP_PROTO_HASHTABLE_VALUE desc_php = get_ce_obj(PHP_PROTO_CE_UNREF(pce)); + if (desc_php == NULL) { + EnumDescriptorInternal* intern = get_ce_enumdesc(PHP_PROTO_CE_UNREF(pce)); + if (intern == NULL) { + RETURN_NULL(); + } + +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(desc_php); + ZVAL_OBJ(desc_php, enum_descriptor_type->create_object( + enum_descriptor_type TSRMLS_CC)); + Z_DELREF_P(desc_php); +#else + desc_php = + enum_descriptor_type->create_object(enum_descriptor_type TSRMLS_CC); + GC_DELREF(desc_php); +#endif + EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE(EnumDescriptor, desc_php); + desc->intern = intern; + add_ce_obj(PHP_PROTO_CE_UNREF(pce), desc_php); } - zend_class_entry* instance_ce = HASHTABLE_VALUE_CE(desc); + zend_class_entry* instance_ce = HASHTABLE_VALUE_CE(desc_php); if (!instanceof_function(instance_ce, enum_descriptor_type TSRMLS_CC)) { RETURN_NULL(); } #if PHP_MAJOR_VERSION < 7 - RETURN_ZVAL(desc, 1, 0); + RETURN_ZVAL(desc_php, 1, 0); #else - GC_ADDREF(desc); - RETURN_OBJ(desc); + GC_ADDREF(desc_php); + RETURN_OBJ(desc_php); #endif } From f33ddb934c62688f4b2a8236177fbf46ca09dcb6 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Fri, 15 Nov 2019 23:40:25 +0000 Subject: [PATCH 10/16] Fix bug --- php/ext/google/protobuf/def.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index a7cf36da3f19..9fa5abfff1ec 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -460,6 +460,7 @@ PHP_METHOD(FieldDescriptor, getEnumType) { EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE(EnumDescriptor, desc_php); desc->intern = intern; add_def_obj(enumdef, desc_php); + add_ce_obj(intern->klass, desc_php); } #if PHP_MAJOR_VERSION < 7 @@ -497,6 +498,7 @@ PHP_METHOD(FieldDescriptor, getMessageType) { Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); desc->intern = intern; add_def_obj(msgdef, desc_php); + add_ce_obj(intern->klass, desc_php); } #if PHP_MAJOR_VERSION < 7 @@ -1096,6 +1098,7 @@ PHP_METHOD(DescriptorPool, getDescriptorByClassName) { #endif Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, desc_php); desc->intern = intern; + add_def_obj(intern->msgdef, desc_php); add_ce_obj(PHP_PROTO_CE_UNREF(pce), desc_php); } @@ -1150,6 +1153,7 @@ PHP_METHOD(DescriptorPool, getEnumDescriptorByClassName) { #endif EnumDescriptor* desc = UNBOX_HASHTABLE_VALUE(EnumDescriptor, desc_php); desc->intern = intern; + add_def_obj(intern->enumdef, desc_php); add_ce_obj(PHP_PROTO_CE_UNREF(pce), desc_php); } From 27130775213f1b774abd167ddceb65cdee8a8c8b Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sat, 16 Nov 2019 00:02:54 +0000 Subject: [PATCH 11/16] Avoid add generated file again if it has been added --- php/ext/google/protobuf/def.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 9fa5abfff1ec..b761d2f47bfd 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -950,14 +950,22 @@ const upb_filedef *parse_and_add_descriptor(const char *data, if (!set) { zend_error(E_ERROR, "Failed to parse binary descriptor\n"); - return false; + return NULL; } files = google_protobuf_FileDescriptorSet_file(set, &n); if (n != 1) { zend_error(E_ERROR, "Serialized descriptors should have exactly one file"); - return false; + return NULL; + } + + // Check whether file has already been added. + upb_strview name = google_protobuf_FileDescriptorProto_name(files[0]); + // TODO(teboring): Needs another look up method which takes data and length. + file = upb_symtab_lookupfile(pool->symtab, name.data); + if (file != NULL) { + return NULL; } // The PHP code generator currently special-cases descriptor.proto. It @@ -968,7 +976,7 @@ const upb_filedef *parse_and_add_descriptor(const char *data, NULL) { if (!parse_and_add_descriptor((char *)descriptor_proto, descriptor_proto_len, pool, arena)) { - return false; + return NULL; } } From ab6a80a719ec058018c6fb939238dbe946abb440 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sat, 16 Nov 2019 00:30:04 +0000 Subject: [PATCH 12/16] Fix the bug upb depends on null-ended str for look up. --- php/ext/google/protobuf/def.c | 2 +- php/ext/google/protobuf/protobuf.c | 20 ++++++++++---------- php/ext/google/protobuf/upb.c | 7 +++++++ php/ext/google/protobuf/upb.h | 2 ++ php/tests/gdb_test.sh | 3 +-- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index b761d2f47bfd..fff62915dbdb 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -963,7 +963,7 @@ const upb_filedef *parse_and_add_descriptor(const char *data, // Check whether file has already been added. upb_strview name = google_protobuf_FileDescriptorProto_name(files[0]); // TODO(teboring): Needs another look up method which takes data and length. - file = upb_symtab_lookupfile(pool->symtab, name.data); + file = upb_symtab_lookupfile2(pool->symtab, name.data, name.size); if (file != NULL) { return NULL; } diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 8e3b9db6b143..ee4bbd2eea1a 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -391,16 +391,16 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { } #endif - is_inited_file_any = true; - is_inited_file_api = true; - is_inited_file_duration = true; - is_inited_file_field_mask = true; - is_inited_file_empty = true; - is_inited_file_source_context = true; - is_inited_file_struct = true; - is_inited_file_timestamp = true; - is_inited_file_type = true; - is_inited_file_wrappers = true; + // is_inited_file_any = true; + // is_inited_file_api = true; + // is_inited_file_duration = true; + // is_inited_file_field_mask = true; + // is_inited_file_empty = true; + // is_inited_file_source_context = true; + // is_inited_file_struct = true; + // is_inited_file_timestamp = true; + // is_inited_file_type = true; + // is_inited_file_wrappers = true; return 0; } diff --git a/php/ext/google/protobuf/upb.c b/php/ext/google/protobuf/upb.c index 9ec5bdf289d4..e69544755b97 100644 --- a/php/ext/google/protobuf/upb.c +++ b/php/ext/google/protobuf/upb.c @@ -4589,6 +4589,13 @@ const upb_filedef *upb_symtab_lookupfile(const upb_symtab *s, const char *name) : NULL; } +const upb_filedef *upb_symtab_lookupfile2( + const upb_symtab *s, const char *name, size_t len) { + upb_value v; + return upb_strtable_lookup2(&s->files, name, len, &v) ? + upb_value_getconstptr(v) : NULL; +} + const upb_filedef *upb_symtab_addfile( upb_symtab *s, const google_protobuf_FileDescriptorProto *file_proto, upb_status *status) { diff --git a/php/ext/google/protobuf/upb.h b/php/ext/google/protobuf/upb.h index 058772741674..1b0075c2a527 100644 --- a/php/ext/google/protobuf/upb.h +++ b/php/ext/google/protobuf/upb.h @@ -3760,6 +3760,8 @@ const upb_msgdef *upb_symtab_lookupmsg2( const upb_symtab *s, const char *sym, size_t len); const upb_enumdef *upb_symtab_lookupenum(const upb_symtab *s, const char *sym); const upb_filedef *upb_symtab_lookupfile(const upb_symtab *s, const char *name); +const upb_filedef *upb_symtab_lookupfile2( + const upb_symtab *s, const char *name, size_t len); int upb_symtab_filecount(const upb_symtab *s); const upb_filedef *upb_symtab_addfile( upb_symtab *s, const google_protobuf_FileDescriptorProto *file, diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index da109f4efcec..da5f3f3ac14b 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -12,8 +12,7 @@ php -i | grep "Configuration" # 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 well_known_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 0dcd5e7d8074dbeee301990957db70319392b318 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sat, 16 Nov 2019 00:58:04 +0000 Subject: [PATCH 13/16] Initialize generated pool impl --- php/ext/google/protobuf/def.c | 27 ++++++++++++++++++++++++++- php/ext/google/protobuf/protobuf.c | 3 +++ php/ext/google/protobuf/protobuf.h | 22 +++++++++++++++++++++- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index fff62915dbdb..ccbbc94a4d90 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -619,7 +619,8 @@ zval* internal_generated_pool_php; zend_object *generated_pool_php; zend_object *internal_generated_pool_php; #endif -InternalDescriptorPool *generated_pool; // The actual generated pool +InternalDescriptorPool *generated_pool; +InternalDescriptorPoolImpl generated_pool_impl; // The actual generated pool void init_generated_pool_once(TSRMLS_D) { if (generated_pool == NULL) { @@ -645,6 +646,7 @@ void init_generated_pool_once(TSRMLS_D) { static void internal_descriptor_pool_init_c_instance( InternalDescriptorPool *pool TSRMLS_DC) { + pool->intern = &generated_pool_impl; pool->symtab = upb_symtab_new(); pool->fill_handler_cache = upb_handlercache_new(add_handlers_for_message, NULL); @@ -666,6 +668,29 @@ static void internal_descriptor_pool_free_c( upb_json_codecache_free(pool->json_fill_method_cache); } +void internal_descriptor_pool_impl_init( + InternalDescriptorPoolImpl *pool TSRMLS_DC) { + pool->symtab = upb_symtab_new(); + pool->fill_handler_cache = + upb_handlercache_new(add_handlers_for_message, NULL); + pool->pb_serialize_handler_cache = upb_pb_encoder_newcache(); + pool->json_serialize_handler_cache = upb_json_printer_newcache(false); + pool->json_serialize_handler_preserve_cache = upb_json_printer_newcache(true); + pool->fill_method_cache = upb_pbcodecache_new(pool->fill_handler_cache); + pool->json_fill_method_cache = upb_json_codecache_new(); +} + +void internal_descriptor_pool_impl_destroy( + InternalDescriptorPoolImpl *pool TSRMLS_DC) { + upb_symtab_free(pool->symtab); + upb_handlercache_free(pool->fill_handler_cache); + upb_handlercache_free(pool->pb_serialize_handler_cache); + upb_handlercache_free(pool->json_serialize_handler_cache); + upb_handlercache_free(pool->json_serialize_handler_preserve_cache); + upb_pbcodecache_free(pool->fill_method_cache); + upb_json_codecache_free(pool->json_fill_method_cache); +} + static void descriptor_pool_init_c_instance(DescriptorPool *pool TSRMLS_DC) { assert(generated_pool != NULL); pool->intern = generated_pool; diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index ee4bbd2eea1a..63e09992e9dd 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -427,6 +427,7 @@ static PHP_MINIT_FUNCTION(protobuf) { upb_strtable_init(&reserved_names, UPB_CTYPE_UINT64); reserved_names_init(); + internal_descriptor_pool_impl_init(&generated_pool_impl TSRMLS_CC); descriptor_pool_init(TSRMLS_C); descriptor_init(TSRMLS_C); @@ -522,6 +523,8 @@ static PHP_MSHUTDOWN_FUNCTION(protobuf) { cleanup_desc_table(&upb_def_to_desc_map_persistent); cleanup_enumdesc_table(&upb_def_to_enumdesc_map_persistent); + internal_descriptor_pool_impl_destroy(&generated_pool_impl TSRMLS_CC); + upb_inttable_uninit(&upb_def_to_desc_map_persistent); upb_inttable_uninit(&upb_def_to_enumdesc_map_persistent); upb_inttable_uninit(&ce_to_desc_map_persistent); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 663f0d4d8b1c..f0d952e999b8 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -609,6 +609,7 @@ struct GPBEmpty; struct Int32Value; struct Int64Value; struct InternalDescriptorPool; +struct InternalDescriptorPoolImpl; struct ListValue; struct Map; struct MapIter; @@ -656,6 +657,7 @@ typedef struct GPBEmpty GPBEmpty; typedef struct Int32Value Int32Value; typedef struct Int64Value Int64Value; typedef struct InternalDescriptorPool InternalDescriptorPool; +typedef struct InternalDescriptorPoolImpl InternalDescriptorPoolImpl; typedef struct ListValue ListValue; typedef struct MapIter MapIter; typedef struct Map Map; @@ -792,7 +794,18 @@ PHP_METHOD(DescriptorPool, getGeneratedPool); PHP_METHOD(DescriptorPool, getDescriptorByClassName); PHP_METHOD(DescriptorPool, getEnumDescriptorByClassName); +struct InternalDescriptorPoolImpl { + upb_symtab* symtab; + upb_handlercache* fill_handler_cache; + upb_handlercache* pb_serialize_handler_cache; + upb_handlercache* json_serialize_handler_cache; + upb_handlercache* json_serialize_handler_preserve_cache; + upb_pbcodecache* fill_method_cache; + upb_json_codecache* json_fill_method_cache; +}; + PHP_PROTO_WRAP_OBJECT_START(InternalDescriptorPool) + InternalDescriptorPoolImpl* intern; upb_symtab* symtab; upb_handlercache* fill_handler_cache; upb_handlercache* pb_serialize_handler_cache; @@ -823,7 +836,14 @@ extern zend_object *internal_generated_pool_php; void descriptor_pool_free(zend_object* object); void internal_descriptor_pool_free(zend_object* object); #endif -extern InternalDescriptorPool* generated_pool; // The actual generated pool +extern InternalDescriptorPool* generated_pool; +// The actual generated pool +extern InternalDescriptorPoolImpl generated_pool_impl; + +void internal_descriptor_pool_impl_init( + InternalDescriptorPoolImpl *pool TSRMLS_DC); +void internal_descriptor_pool_impl_destroy( + InternalDescriptorPoolImpl *pool TSRMLS_DC); struct DescriptorInternal { InternalDescriptorPool* pool; From ea45603f735e9f008532d19f5947170f52f361cb Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sat, 16 Nov 2019 01:17:17 +0000 Subject: [PATCH 14/16] Turn down old generated pool --- php/ext/google/protobuf/def.c | 33 +++++------------------------- php/ext/google/protobuf/protobuf.c | 20 +++++++++--------- php/ext/google/protobuf/protobuf.h | 13 +++--------- 3 files changed, 18 insertions(+), 48 deletions(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index ccbbc94a4d90..7c575c35cfe8 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -619,7 +619,7 @@ zval* internal_generated_pool_php; zend_object *generated_pool_php; zend_object *internal_generated_pool_php; #endif -InternalDescriptorPool *generated_pool; +InternalDescriptorPoolImpl *generated_pool; InternalDescriptorPoolImpl generated_pool_impl; // The actual generated pool void init_generated_pool_once(TSRMLS_D) { @@ -630,42 +630,25 @@ void init_generated_pool_once(TSRMLS_D) { ZVAL_OBJ(internal_generated_pool_php, internal_descriptor_pool_type->create_object( internal_descriptor_pool_type TSRMLS_CC)); - generated_pool = UNBOX(InternalDescriptorPool, internal_generated_pool_php); ZVAL_OBJ(generated_pool_php, descriptor_pool_type->create_object( descriptor_pool_type TSRMLS_CC)); #else internal_generated_pool_php = internal_descriptor_pool_type->create_object( internal_descriptor_pool_type TSRMLS_CC); - generated_pool = (InternalDescriptorPool *)((char *)internal_generated_pool_php - - XtOffsetOf(InternalDescriptorPool, std)); generated_pool_php = descriptor_pool_type->create_object(descriptor_pool_type TSRMLS_CC); #endif + generated_pool = &generated_pool_impl; } } static void internal_descriptor_pool_init_c_instance( InternalDescriptorPool *pool TSRMLS_DC) { pool->intern = &generated_pool_impl; - pool->symtab = upb_symtab_new(); - pool->fill_handler_cache = - upb_handlercache_new(add_handlers_for_message, NULL); - pool->pb_serialize_handler_cache = upb_pb_encoder_newcache(); - pool->json_serialize_handler_cache = upb_json_printer_newcache(false); - pool->json_serialize_handler_preserve_cache = upb_json_printer_newcache(true); - pool->fill_method_cache = upb_pbcodecache_new(pool->fill_handler_cache); - pool->json_fill_method_cache = upb_json_codecache_new(); } static void internal_descriptor_pool_free_c( InternalDescriptorPool *pool TSRMLS_DC) { - upb_symtab_free(pool->symtab); - upb_handlercache_free(pool->fill_handler_cache); - upb_handlercache_free(pool->pb_serialize_handler_cache); - upb_handlercache_free(pool->json_serialize_handler_cache); - upb_handlercache_free(pool->json_serialize_handler_preserve_cache); - upb_pbcodecache_free(pool->fill_method_cache); - upb_json_codecache_free(pool->json_fill_method_cache); } void internal_descriptor_pool_impl_init( @@ -962,7 +945,7 @@ bool depends_on_descriptor(const google_protobuf_FileDescriptorProto* file) { const upb_filedef *parse_and_add_descriptor(const char *data, PHP_PROTO_SIZE data_len, - InternalDescriptorPool *pool, + InternalDescriptorPoolImpl *pool, upb_arena *arena) { size_t n; google_protobuf_FileDescriptorSet *set; @@ -1012,7 +995,7 @@ const upb_filedef *parse_and_add_descriptor(const char *data, } void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len, - InternalDescriptorPool *pool, + InternalDescriptorPoolImpl *pool, bool use_nested_submsg TSRMLS_DC) { int i; upb_arena *arena; @@ -1090,14 +1073,11 @@ PHP_METHOD(InternalDescriptorPool, internalAddGeneratedFile) { } InternalDescriptorPool *pool = UNBOX(InternalDescriptorPool, getThis()); - internal_add_generated_file(data, data_len, pool, + internal_add_generated_file(data, data_len, pool->intern, use_nested_submsg TSRMLS_CC); } PHP_METHOD(DescriptorPool, getDescriptorByClassName) { - DescriptorPool *public_pool = UNBOX(DescriptorPool, getThis()); - InternalDescriptorPool *pool = public_pool->intern; - char *classname = NULL; PHP_PROTO_SIZE classname_len; @@ -1150,9 +1130,6 @@ PHP_METHOD(DescriptorPool, getDescriptorByClassName) { } PHP_METHOD(DescriptorPool, getEnumDescriptorByClassName) { - DescriptorPool *public_pool = UNBOX(DescriptorPool, getThis()); - InternalDescriptorPool *pool = public_pool->intern; - char *classname = NULL; PHP_PROTO_SIZE classname_len; diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 63e09992e9dd..98ea1e2c0e80 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -391,16 +391,16 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { } #endif - // is_inited_file_any = true; - // is_inited_file_api = true; - // is_inited_file_duration = true; - // is_inited_file_field_mask = true; - // is_inited_file_empty = true; - // is_inited_file_source_context = true; - // is_inited_file_struct = true; - // is_inited_file_timestamp = true; - // is_inited_file_type = true; - // is_inited_file_wrappers = true; + is_inited_file_any = true; + is_inited_file_api = true; + is_inited_file_duration = true; + is_inited_file_field_mask = true; + is_inited_file_empty = true; + is_inited_file_source_context = true; + is_inited_file_struct = true; + is_inited_file_timestamp = true; + is_inited_file_type = true; + is_inited_file_wrappers = true; return 0; } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index f0d952e999b8..f8e4be079508 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -806,20 +806,13 @@ struct InternalDescriptorPoolImpl { PHP_PROTO_WRAP_OBJECT_START(InternalDescriptorPool) InternalDescriptorPoolImpl* intern; - upb_symtab* symtab; - upb_handlercache* fill_handler_cache; - upb_handlercache* pb_serialize_handler_cache; - upb_handlercache* json_serialize_handler_cache; - upb_handlercache* json_serialize_handler_preserve_cache; - upb_pbcodecache* fill_method_cache; - upb_json_codecache* json_fill_method_cache; PHP_PROTO_WRAP_OBJECT_END PHP_METHOD(InternalDescriptorPool, getGeneratedPool); PHP_METHOD(InternalDescriptorPool, internalAddGeneratedFile); void internal_add_generated_file(const char* data, PHP_PROTO_SIZE data_len, - InternalDescriptorPool* pool, + InternalDescriptorPoolImpl* pool, bool use_nested_submsg TSRMLS_DC); void init_generated_pool_once(TSRMLS_D); void add_handlers_for_message(const void* closure, upb_handlers* h); @@ -836,7 +829,7 @@ extern zend_object *internal_generated_pool_php; void descriptor_pool_free(zend_object* object); void internal_descriptor_pool_free(zend_object* object); #endif -extern InternalDescriptorPool* generated_pool; +extern InternalDescriptorPoolImpl* generated_pool; // The actual generated pool extern InternalDescriptorPoolImpl generated_pool_impl; @@ -846,7 +839,7 @@ void internal_descriptor_pool_impl_destroy( InternalDescriptorPoolImpl *pool TSRMLS_DC); struct DescriptorInternal { - InternalDescriptorPool* pool; + InternalDescriptorPoolImpl* pool; const upb_msgdef* msgdef; MessageLayout* layout; zend_class_entry* klass; // begins as NULL From 1a69c0df3a739135554e4a05dc421be859d74a37 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sun, 17 Nov 2019 19:13:18 +0000 Subject: [PATCH 15/16] Add init entry flag protobuf.keep_descriptor_pool_after_request By default, it's off. Add protobuf.keep_descriptor_pool_after_request=1 to php.ini to enable it --- php/ext/google/protobuf/protobuf.c | 156 ++++++++++++++++------------- php/ext/google/protobuf/protobuf.h | 1 + php/tests/test.sh | 10 ++ 3 files changed, 100 insertions(+), 67 deletions(-) diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 98ea1e2c0e80..8cd4239ee17c 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -337,16 +337,15 @@ static void test_release(void* value) { } #endif -static PHP_RINIT_FUNCTION(protobuf) { - ALLOC_HASHTABLE(upb_def_to_php_obj_map); - zend_hash_init(upb_def_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); - - ALLOC_HASHTABLE(ce_to_php_obj_map); - zend_hash_init(ce_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); +static initialize_persistent_descriptor_pool() { + upb_inttable_init(&upb_def_to_desc_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&upb_def_to_enumdesc_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&ce_to_desc_map_persistent, UPB_CTYPE_PTR); + upb_inttable_init(&ce_to_enumdesc_map_persistent, UPB_CTYPE_PTR); + upb_strtable_init(&proto_to_desc_map_persistent, UPB_CTYPE_PTR); + upb_strtable_init(&proto_to_enumdesc_map_persistent, UPB_CTYPE_PTR); - generated_pool = NULL; - generated_pool_php = NULL; - internal_generated_pool_php = NULL; + internal_descriptor_pool_impl_init(&generated_pool_impl TSRMLS_CC); is_inited_file_any = false; is_inited_file_api = false; @@ -358,10 +357,72 @@ static PHP_RINIT_FUNCTION(protobuf) { is_inited_file_timestamp = false; is_inited_file_type = false; is_inited_file_wrappers = false; +} + +static PHP_RINIT_FUNCTION(protobuf) { + ALLOC_HASHTABLE(upb_def_to_php_obj_map); + zend_hash_init(upb_def_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); + + ALLOC_HASHTABLE(ce_to_php_obj_map); + zend_hash_init(ce_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); + + generated_pool = NULL; + generated_pool_php = NULL; + internal_generated_pool_php = NULL; + + if (!PROTOBUF_G(keep_descriptor_pool_after_request)) { + initialize_persistent_descriptor_pool(); + } return 0; } +static void cleanup_desc_table(upb_inttable* t) { + upb_inttable_iter i; + upb_value v; + DescriptorInternal* desc; + for(upb_inttable_begin(&i, t); + !upb_inttable_done(&i); + upb_inttable_next(&i)) { + v = upb_inttable_iter_value(&i); + desc = upb_value_getptr(v); + if (desc->layout) { + free_layout(desc->layout); + } + SYS_FREE(desc); + } +} + +static void cleanup_enumdesc_table(upb_inttable* t) { + upb_inttable_iter i; + upb_value v; + EnumDescriptorInternal* desc; + for(upb_inttable_begin(&i, t); + !upb_inttable_done(&i); + upb_inttable_next(&i)) { + v = upb_inttable_iter_value(&i); + desc = upb_value_getptr(v); + SYS_FREE(desc); + } +} + +static cleanup_persistent_descriptor_pool() { + // Clean up + + // Only needs to clean one map out of three (def=>desc, ce=>desc, proto=>desc) + cleanup_desc_table(&upb_def_to_desc_map_persistent); + cleanup_enumdesc_table(&upb_def_to_enumdesc_map_persistent); + + internal_descriptor_pool_impl_destroy(&generated_pool_impl TSRMLS_CC); + + upb_inttable_uninit(&upb_def_to_desc_map_persistent); + upb_inttable_uninit(&upb_def_to_enumdesc_map_persistent); + upb_inttable_uninit(&ce_to_desc_map_persistent); + upb_inttable_uninit(&ce_to_enumdesc_map_persistent); + upb_strtable_uninit(&proto_to_desc_map_persistent); + upb_strtable_uninit(&proto_to_enumdesc_map_persistent); +} + static PHP_RSHUTDOWN_FUNCTION(protobuf) { zend_hash_destroy(upb_def_to_php_obj_map); FREE_HASHTABLE(upb_def_to_php_obj_map); @@ -391,16 +452,9 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { } #endif - is_inited_file_any = true; - is_inited_file_api = true; - is_inited_file_duration = true; - is_inited_file_field_mask = true; - is_inited_file_empty = true; - is_inited_file_source_context = true; - is_inited_file_struct = true; - is_inited_file_timestamp = true; - is_inited_file_type = true; - is_inited_file_wrappers = true; + if (!PROTOBUF_G(keep_descriptor_pool_after_request)) { + cleanup_persistent_descriptor_pool(); + } return 0; } @@ -417,17 +471,22 @@ static void reserved_names_init() { } } +PHP_INI_BEGIN() +STD_PHP_INI_ENTRY("protobuf.keep_descriptor_pool_after_request", "0", + PHP_INI_SYSTEM, OnUpdateBool, + keep_descriptor_pool_after_request, zend_protobuf_globals, + protobuf_globals) +PHP_INI_END() + static PHP_MINIT_FUNCTION(protobuf) { - upb_inttable_init(&upb_def_to_desc_map_persistent, UPB_CTYPE_PTR); - upb_inttable_init(&upb_def_to_enumdesc_map_persistent, UPB_CTYPE_PTR); - upb_inttable_init(&ce_to_desc_map_persistent, UPB_CTYPE_PTR); - upb_inttable_init(&ce_to_enumdesc_map_persistent, UPB_CTYPE_PTR); - upb_strtable_init(&proto_to_desc_map_persistent, UPB_CTYPE_PTR); - upb_strtable_init(&proto_to_enumdesc_map_persistent, UPB_CTYPE_PTR); - upb_strtable_init(&reserved_names, UPB_CTYPE_UINT64); + REGISTER_INI_ENTRIES(); + upb_strtable_init(&reserved_names, UPB_CTYPE_UINT64); reserved_names_init(); - internal_descriptor_pool_impl_init(&generated_pool_impl TSRMLS_CC); + + if (PROTOBUF_G(keep_descriptor_pool_after_request)) { + initialize_persistent_descriptor_pool(); + } descriptor_pool_init(TSRMLS_C); descriptor_init(TSRMLS_C); @@ -489,48 +548,11 @@ static PHP_MINIT_FUNCTION(protobuf) { return 0; } -static void cleanup_desc_table(upb_inttable* t) { - upb_inttable_iter i; - upb_value v; - DescriptorInternal* desc; - for(upb_inttable_begin(&i, t); - !upb_inttable_done(&i); - upb_inttable_next(&i)) { - v = upb_inttable_iter_value(&i); - desc = upb_value_getptr(v); - if (desc->layout) { - free_layout(desc->layout); - } - SYS_FREE(desc); - } -} - -static void cleanup_enumdesc_table(upb_inttable* t) { - upb_inttable_iter i; - upb_value v; - EnumDescriptorInternal* desc; - for(upb_inttable_begin(&i, t); - !upb_inttable_done(&i); - upb_inttable_next(&i)) { - v = upb_inttable_iter_value(&i); - desc = upb_value_getptr(v); - SYS_FREE(desc); - } -} - static PHP_MSHUTDOWN_FUNCTION(protobuf) { - // Only needs to clean one map out of three (def=>desc, ce=>desc, proto=>desc) - cleanup_desc_table(&upb_def_to_desc_map_persistent); - cleanup_enumdesc_table(&upb_def_to_enumdesc_map_persistent); - - internal_descriptor_pool_impl_destroy(&generated_pool_impl TSRMLS_CC); + if (PROTOBUF_G(keep_descriptor_pool_after_request)) { + cleanup_persistent_descriptor_pool(); + } - upb_inttable_uninit(&upb_def_to_desc_map_persistent); - upb_inttable_uninit(&upb_def_to_enumdesc_map_persistent); - upb_inttable_uninit(&ce_to_desc_map_persistent); - upb_inttable_uninit(&ce_to_enumdesc_map_persistent); - upb_strtable_uninit(&proto_to_desc_map_persistent); - upb_strtable_uninit(&proto_to_enumdesc_map_persistent); upb_strtable_uninit(&reserved_names); PEFREE(message_handlers); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index f8e4be079508..e35a99d6b1ef 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -686,6 +686,7 @@ typedef struct Value Value; // ----------------------------------------------------------------------------- ZEND_BEGIN_MODULE_GLOBALS(protobuf) + zend_bool keep_descriptor_pool_after_request; ZEND_END_MODULE_GLOBALS(protobuf) ZEND_DECLARE_MODULE_GLOBALS(protobuf) diff --git a/php/tests/test.sh b/php/tests/test.sh index be6e97fe8594..9ef565c71338 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -20,12 +20,22 @@ do echo "" done +for t in "${tests[@]}" +do + echo "****************************" + echo "* $t persistent" + echo "****************************" + php -d protobuf.keep_descriptor_pool_after_request=1 -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t + echo "" +done + # # Make sure to run the memory test in debug mode. # php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php export ZEND_DONT_UNLOAD_MODULES=1 export USE_ZEND_ALLOC=0 valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php +valgrind --leak-check=yes php -d protobuf.keep_descriptor_pool_after_request=1 -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # TODO(teboring): Only for debug (phpunit has memory leak which blocks this beging used by # regresssion test.) From a350c05993a35e46eb3a107d13a47458349a423b Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sun, 17 Nov 2019 19:33:05 +0000 Subject: [PATCH 16/16] Fix zts build --- php/ext/google/protobuf/protobuf.c | 12 ++++++------ php/ext/google/protobuf/protobuf.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 8cd4239ee17c..fd97c89bd73f 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -337,7 +337,7 @@ static void test_release(void* value) { } #endif -static initialize_persistent_descriptor_pool() { +static initialize_persistent_descriptor_pool(TSRMLS_D) { upb_inttable_init(&upb_def_to_desc_map_persistent, UPB_CTYPE_PTR); upb_inttable_init(&upb_def_to_enumdesc_map_persistent, UPB_CTYPE_PTR); upb_inttable_init(&ce_to_desc_map_persistent, UPB_CTYPE_PTR); @@ -371,7 +371,7 @@ static PHP_RINIT_FUNCTION(protobuf) { internal_generated_pool_php = NULL; if (!PROTOBUF_G(keep_descriptor_pool_after_request)) { - initialize_persistent_descriptor_pool(); + initialize_persistent_descriptor_pool(TSRMLS_C); } return 0; @@ -406,7 +406,7 @@ static void cleanup_enumdesc_table(upb_inttable* t) { } } -static cleanup_persistent_descriptor_pool() { +static cleanup_persistent_descriptor_pool(TSRMLS_D) { // Clean up // Only needs to clean one map out of three (def=>desc, ce=>desc, proto=>desc) @@ -453,7 +453,7 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { #endif if (!PROTOBUF_G(keep_descriptor_pool_after_request)) { - cleanup_persistent_descriptor_pool(); + cleanup_persistent_descriptor_pool(TSRMLS_C); } return 0; @@ -485,7 +485,7 @@ static PHP_MINIT_FUNCTION(protobuf) { reserved_names_init(); if (PROTOBUF_G(keep_descriptor_pool_after_request)) { - initialize_persistent_descriptor_pool(); + initialize_persistent_descriptor_pool(TSRMLS_C); } descriptor_pool_init(TSRMLS_C); @@ -550,7 +550,7 @@ static PHP_MINIT_FUNCTION(protobuf) { static PHP_MSHUTDOWN_FUNCTION(protobuf) { if (PROTOBUF_G(keep_descriptor_pool_after_request)) { - cleanup_persistent_descriptor_pool(); + cleanup_persistent_descriptor_pool(TSRMLS_C); } upb_strtable_uninit(&reserved_names); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index e35a99d6b1ef..08ce131da835 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -788,7 +788,7 @@ extern zend_class_entry* repeated_field_type; // ----------------------------------------------------------------------------- PHP_PROTO_WRAP_OBJECT_START(DescriptorPool) - InternalDescriptorPool* intern; + InternalDescriptorPoolImpl* intern; PHP_PROTO_WRAP_OBJECT_END PHP_METHOD(DescriptorPool, getGeneratedPool);