Lazily Create Singular Wrapper Message #6833
Merged
Conversation
This file contains 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
Need check type of field before getting submsg def
TeBoring
changed the title
* Update upb to fix Timestamp conformance tests * Fix segmentation fault for oneof wrapper fields
…ess wrapper value
TeBoring
changed the title
stanley-cheung
approved these changes
Nov 13, 2019
There was a problem hiding this comment.
Can you also document the motivation of this PR please (a few paragraphs in the PR summary would suffice)? It seems that it has to do with the usage of well known wrapper to replace primitive fields, and to improve performance for object creation by lazily creating these wrapper objects. But it isn't clear from just the PR title.
Please also document that there are protoc code generator changes but things are backward compatible.
nlhien
pushed a commit
to nlhien/protobuf
that referenced
this pull request
Feb 28, 2020
* Register additional handlers from wrappers * Return zval instead of parse frame * Use parse frame * Update upb * Lazily create wrapper messages * Fix a segment fault Need check type of field before getting submsg def * Avoid expanding during serialization and direct access * Fix a bug that getXXXUnwrapped returns null for string * Implement writeWrapperUnwrapped * Add more tests * Fix oneof wrapper parsing * Fix get oneof field * Avoid expansion for oneof wrappers * Fix bug * Fix a bug that in php7 variable is defined out of scope * Fix broken tests * Update upb to fix Timestamp conformance tests * Fix segmentation fault for oneof wrapper fields * Fix encoding/decoding top level wrapper values * Add type checking for write wrapper value in php7 * Fix zts build * Fix the bug that readWrapperValue uses parent message's layout to access wrapper value * Fix wrapper in map
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In order to mimic proto2 presence semantic in proto3, some users use lots of well known wrapper to replace primitive fields. This causes the problem of performance, because object creation is expensive.
This change lazily creates wrapper objects, which saves lots of time for unnecessary object creation/deletion.
In order to use the new feature, users needs to use the protoc in this change to update their generated code. However, old generated code is still compatible with the new runtime.