Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement lazy loading of php class for proto messages #6911

Merged
merged 5 commits into from Nov 21, 2019

Conversation

TeBoring
Copy link
Contributor

@TeBoring TeBoring commented Nov 20, 2019

Previously, when creating a message, all messages defined in the same proto file and transitive proto files need to be resolved by php engine, even though some of them are not needed.
This change lazily resolve php classes of proto messages, so that they are only needed if the actual message is created.

@TeBoring TeBoring added php release notes: yes kokoro:run labels Nov 20, 2019
@TeBoring TeBoring changed the title Implement lazy loading of php class for proto messages [WIP] Implement lazy loading of php class for proto messages Nov 20, 2019
@@ -1356,15 +1366,6 @@ void add_handlers_for_message(const void* closure, upb_handlers* h) {
return;
}

// Ensure layout exists. We may be invoked to create handlers for a given
Copy link
Contributor

@stanley-cheung stanley-cheung Nov 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we can remove this

Copy link
Contributor Author

@TeBoring TeBoring Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been done in register_class

@@ -817,6 +821,7 @@ void internal_add_generated_file(const char* data, PHP_PROTO_SIZE data_len,
bool use_nested_submsg TSRMLS_DC);
void init_generated_pool_once(TSRMLS_D);
void add_handlers_for_message(const void* closure, upb_handlers* h);
void register_class(void *desc, bool is_enum TSRMLS_DC);
Copy link
Contributor

@stanley-cheung stanley-cheung Nov 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is register_class a function that already existed before this PR?

Copy link
Contributor Author

@TeBoring TeBoring Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It existed as a static method in def.c.
In this change, I refactored a little to make it easy to use.

@TeBoring TeBoring merged commit 6d7bb7e into protocolbuffers:master Nov 21, 2019
56 checks passed
rafi-kamal pushed a commit to rafi-kamal/protobuf that referenced this issue Nov 21, 2019
…rs#6911)

* Implement lazy loading of php class for proto messages

* Fix php 7.1

* Fix encode

* Fix memory leak

* Fix enum descriptor
rafi-kamal added a commit that referenced this issue Nov 22, 2019
* Implement lazy loading of php class for proto messages

* Fix php 7.1

* Fix encode

* Fix memory leak

* Fix enum descriptor
nlhien pushed a commit to nlhien/protobuf that referenced this issue Feb 28, 2020
…rs#6911)

* Implement lazy loading of php class for proto messages

* Fix php 7.1

* Fix encode

* Fix memory leak

* Fix enum descriptor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes php release notes: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants