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

Make project config cache compatible #486

Merged
merged 7 commits into from
Jul 20, 2023
Merged

Make project config cache compatible #486

merged 7 commits into from
Jul 20, 2023

Conversation

devPalacio
Copy link
Contributor

@devPalacio devPalacio commented Jan 22, 2023

This PR moves us to the Gradle APIs which enable configuration avoidance (aka lazy config). After the refactor, I made sure no configuration cache problems appeared when running tasks in the project.

  • The generateStone task was modified to properly type task inputs and outputs.
  • Gradle is upgraded to 7.6.2
  • Maven publish plugin updated to match the version we use internally
  • AGP upgraded to 7.4.2 for config cache fixes

Measured improvements with gradle-profiler:

  • Running help (to measure configuration time) went from 236 ms => 59 ms
  • Running a clean :generateStone (actually deleting generated_stone_source) went from ~7 s to ~1 s (mainly due to turning on build-cache)

@@ -85,14 +87,16 @@ processResources { task ->
}
}

compileJava {
tasks.named("compileJava", JavaCompile) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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


# Use AndroidX
android.useAndroidX=true

org.gradle.jvmargs=-Xmx2048m -XX:MaxPermSize=512m -Dfile.encoding=UTF-8
Copy link
Contributor Author

@devPalacio devPalacio Jan 22, 2023

Choose a reason for hiding this comment

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

https://docs.oracle.com/javase/9/migrate/
The permanent generation was removed in JDK 8, this option causes gradle to fail in newer JDK versions with the following error.

Unable to start the daemon process.
This problem might be caused by incorrect configuration of the daemon.
For example, an unrecognized jvm option is used.
Unrecognized VM option 'MaxPermSize=512m'

# Disable Configuration Cache until we test it in this project
org.gradle.unsafe.configuration-cache=false
org.gradle.unsafe.configuration-cache=true
org.gradle.caching=true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

enabling build cache should help speed things up as well

@@ -21,20 +21,6 @@ plugins {
alias(dropboxJavaSdkLibs.plugins.binary.compatibility.validator) apply false
}

allprojects {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to root settings.gradle to centralize repo declaration

mavenCentral()
}

plugins.withId("com.vanniktech.maven.publish") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to :dropbox-sdk-java


val compileJava: Task = project.tasks.getByName(sourceSet.getCompileTaskName("java"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These dependency declarations were moved to the top of this file and are applied after the stoneTasks are created.

@@ -1,11 +1,11 @@
[versions]
android-compile-sdk = "33"
android-gradle-plugin = "7.2.2"
android-gradle-plugin = "7.4.2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these bumps were needed for config cache

@devPalacio devPalacio requested review from joshafeinberg and jonamireh and removed request for handstandsam July 19, 2023 20:29
@devPalacio devPalacio self-assigned this Jul 19, 2023
@devPalacio devPalacio marked this pull request as ready for review July 19, 2023 20:34
@devPalacio devPalacio merged commit f463957 into main Jul 20, 2023
2 checks passed
devPalacio added a commit that referenced this pull request Oct 3, 2023
Co-authored-by: Jay Palacio <[email protected]>
(cherry picked from commit f463957)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants