Persistent Descriptor Pool#6899
Merged
TeBoring merged 16 commits intoprotocolbuffers:masterfrom Nov 18, 2019
Merged
Conversation
By default, it's off. Add protobuf.keep_descriptor_pool_after_request=1 to php.ini to enable it
stanley-cheung
approved these changes
Nov 18, 2019
|
|
||
| DescriptorInternal* get_msgdef_desc(const upb_msgdef* m) { | ||
| upb_value v; | ||
| #ifndef NDEBUG |
Contributor
There was a problem hiding this comment.
This seems new, what is this? What does this NDEBUG do?
Contributor
Author
There was a problem hiding this comment.
NDEBUG is a standard macro during compiling, which means no debug mode.
This is somehow exposed by upb. We don't actually depend on it. Just to let upb happy.
| ALLOC_HASHTABLE(proto_to_php_obj_map); | ||
| zend_hash_init(proto_to_php_obj_map, 16, NULL, HASHTABLE_VALUE_DTOR, 0); | ||
| static initialize_persistent_descriptor_pool(TSRMLS_D) { | ||
| upb_inttable_init(&upb_def_to_desc_map_persistent, UPB_CTYPE_PTR); |
Contributor
There was a problem hiding this comment.
What does upb_inttable_init do? This seems central to the theme about creating the persistent pool.
Contributor
Author
There was a problem hiding this comment.
It initializes a custom hashtable implemented by upb.
| internal_generated_pool_php = NULL; | ||
|
|
||
| if (!PROTOBUF_G(keep_descriptor_pool_after_request)) { | ||
| initialize_persistent_descriptor_pool(TSRMLS_C); |
Contributor
There was a problem hiding this comment.
So just to confirm - the logic here is that: if the new php.ini config entry is not set (old behavior), we do this initialize_persistent_descriptor_pool (which I suppose is just current behavior), for every PHP request? (hence this RINIT function.
rafi-kamal
pushed a commit
that referenced
this pull request
Nov 19, 2019
* Make reserve names map persistent * Add DescriptorInternal to map * Use get_msgdef_desc in encode_decode.c * Add persistent map for ce=>def and enum=>def * Replace get_ce_obj * Remove get_proto_obj * Remove obsolete fields from Descriptor and EnumDescriptor * Add cache for descriptor php values * Add cache for descriptors * Fix bug * Avoid add generated file again if it has been added * Fix the bug upb depends on null-ended str for look up. * Initialize generated pool impl * Turn down old generated pool * 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 * Fix zts build
nlhien
pushed a commit
to nlhien/protobuf
that referenced
this pull request
Feb 28, 2020
* Make reserve names map persistent * Add DescriptorInternal to map * Use get_msgdef_desc in encode_decode.c * Add persistent map for ce=>def and enum=>def * Replace get_ce_obj * Remove get_proto_obj * Remove obsolete fields from Descriptor and EnumDescriptor * Add cache for descriptor php values * Add cache for descriptors * Fix bug * Avoid add generated file again if it has been added * Fix the bug upb depends on null-ended str for look up. * Initialize generated pool impl * Turn down old generated pool * 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 * Fix zts build
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, because it's possible that different php requests may have conflicting proto message (different message with the same fully qualified name), it is required that the internal descriptor pool is refreshed after each request, which hurts performance.
This change implements persistent descriptor pool. Users needs to make sure there is no conflicting messages by themselves.
By default, this feature is disabled. To enable it, add
protobuf.keep_descriptor_pool_after_request=1into php.ini. Or on command line, add-d protobuf.keep_descriptor_pool_after_request=1.