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

*: add lite-runtime customization options #399

Closed
wants to merge 2 commits into from

Conversation

BusyJay
Copy link
Contributor

@BusyJay BusyJay commented Mar 13, 2019

Some times, it may not be suitable to enable lite-runtime for all languages.
For java, it even warns about generating broken code. However
rust-protobuf can only enable lite runtime through the default file
option, which makes it difficult to work with different languages.

This pull request provides an additional way to enable the flag, so lite
runtime can be enabled for rust while disabled for all other languages.

Some times, it may not be suitable to enable lite-runtime for all languages.
For java, it even warns about generating broken code. However
rust-protobuf can only enable lite runtime through the default file
option, which make it difficult to work with different languages.

This pull request provides an additional way to enable the flag, so lite
runtime can be enabled for rust while disabled for all other languages.
@stepancheg
Copy link
Owner

I though I have removed lite_runtime options long time ago.

Does it still do something useful? Does it reduce code size significantly?

@BusyJay
Copy link
Contributor Author

BusyJay commented Mar 27, 2019

Yes, it removes more than 10000+ lines of code in our project.

@stepancheg
Copy link
Owner

Is the line count important?

I think the generated machine code size is the largest issue now (and avoiding generation of descriptors reduces generated code size just a little, please correct me if I'm wrong).

This is the issue: #339

Unfortunately, I don't know a good solution to this problem.

@stepancheg
Copy link
Owner

OK, since LITE_RUNTIME is already in stable, there is no harm adding this option.

So I merged this PR (as 1a0c2f4).

However, no promises that lite runtime option will exist in the next big major version.

Thanks!

@stepancheg stepancheg closed this Apr 15, 2019
@stepancheg stepancheg reopened this Apr 15, 2019
@stepancheg
Copy link
Owner

Reopening the PR since this need to be merged to stable branch.

@stepancheg
Copy link
Owner

stepancheg commented Apr 23, 2019

Merged to 2.6 branch.

@stepancheg stepancheg closed this Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants