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

build, examples, README.md: Update protobuf gradle plugin version to 0.8.13 #7355

Merged

Conversation

voidzcy
Copy link
Contributor

@voidzcy voidzcy commented Aug 24, 2020

No description provided.

@voidzcy voidzcy changed the title examples, README.md: Update protobuf gradle plugin version to 0.8.13 (AGP version to 3.5.0) examples, README.md: Update protobuf gradle plugin version to 0.8.13 Aug 24, 2020
@voidzcy voidzcy requested a review from ejona86 August 24, 2020 21:43
@voidzcy
Copy link
Contributor Author

voidzcy commented Aug 25, 2020

Looks the Android build encounters some problem. I will look into it.

@voidzcy voidzcy force-pushed the doc/update_protobuf_gradle_plugin_version_to_0_8_13 branch from 2ef1a1f to 283ad2b Compare August 26, 2020 02:19
@voidzcy voidzcy force-pushed the doc/update_protobuf_gradle_plugin_version_to_0_8_13 branch from b00f04c to e9021f0 Compare September 18, 2020 07:03
@voidzcy
Copy link
Contributor Author

voidzcy commented Sep 18, 2020

There might be significant memory leaks, even 1 GB Metaspace is not enough. Trying 2 GB.

https://source.cloud.google.com/results/invocations/ae418e53-ac04-4eb3-80c6-cde472c90888/targets/grpc%2Fjava%2Fmaster%2Fpresubmit%2Fandroid/log

@voidzcy voidzcy force-pushed the doc/update_protobuf_gradle_plugin_version_to_0_8_13 branch from dc4a687 to ff534be Compare September 18, 2020 19:43
@voidzcy voidzcy force-pushed the doc/update_protobuf_gradle_plugin_version_to_0_8_13 branch from ff534be to c6fa4ff Compare September 18, 2020 21:47
@voidzcy
Copy link
Contributor Author

voidzcy commented Sep 18, 2020

Now it is working fine, with ./gradlew --stop in front of running builds for the previous commit. Basically we force it to use a new daemon for building the previous commit. My thought is the daemon keeps too many class objects with different versions (as this PR is changing) in the Metaspace for building this commit and the previous commit.

Ideally we should not use daemon in production. But building without daemon takes extremely long, which eventually timeout the job.

/cc @ejona86

@voidzcy voidzcy changed the title examples, README.md: Update protobuf gradle plugin version to 0.8.13 build, examples, README.md: Update protobuf gradle plugin version to 0.8.13 Sep 18, 2020
@@ -91,6 +91,7 @@ new_apk_size="$(stat --printf=%s $HELLO_WORLD_OUTPUT_DIR/apk/release/app-release
cd $BASE_DIR/github/grpc-java
git checkout HEAD^
./gradlew clean
./gradlew --stop # use a new daemon to build the previous commit
Copy link
Member

Choose a reason for hiding this comment

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

This should go above the ./gradlew clean since that clean is running at the new commit, or the ./gradlew clean should be moved before the git checkout HEAD^ (preferable).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

Thank you for this. I know it was quite painful.

@voidzcy voidzcy merged commit da100e8 into grpc:master Sep 21, 2020
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
…0.8.13 (grpc#7355)

Updated protobuf gradle plugin version to 0.8.13. Fixed Android Kokoro's memory issue by forcing to use a new Gradle daemon for building the previous commit.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants