jruesga / rview

A Gerrit client application for Android
Apache License 2.0
66 stars 21 forks source link

Gradle: Improved dependencies resolution & run dex in-process #33

Closed sschuberth closed 7 years ago

sschuberth commented 7 years ago

This makes dependencies resolve during Gradle execution time instead of configuration time, speeding up running simple / generic tasks that do not need any dependencies.

Also, increase the Gradle daemon's heap size to be able to run dex in-process.

jruesga commented 7 years ago

I prefer not to submit this optimizations, because:

1.- I didn't observe any improvement from use android.enableImprovedDependenciesResolution in this project, because there are neither multiples apk modules nor flavors, and all the app depedends on all the submodules. In addition, i read of cases causing build problems (doesn't seems to be the case) and it will be activate as default in 2.4.

2.- Although, it should be build running the gradle daemon, IMHO this is a user preference, and the project doesn't have any requirement on this. xe: I'm building with 2040Mb, because I have enough memory to assign to gradle in my build system.

If you need this, I can exclude the gradle.properties from git, so you can have your own build preferences.

sschuberth commented 7 years ago
  1. Regarding enableImprovedDependenciesResolution, you usually see reduced times for e.g. running ./gradlew tasks, but I agree there's no noticeable improvement for this project. I'll drop the commit.

  2. 1536M is the minimum (as suggested by the Android Gradle plugin when building from the command line) to make in-process dexing work, and there is no good reason to stay below that limit. ~If you have more memory and want to use 2040M, then that setting should go into your user-local ~/.gradle/gradle.properties file IMO, as that will take precedence.~ Edit: Scratch that, the project-specific setting takes precedence.

If you agree, I'll create a new PR with only 2., as the branch name is misleading otherwise. If you don't agree and don't want even just 2., feel free to close this unmerged.

jruesga commented 7 years ago

Yes, I prefer to keep it untoched for now. Thank you for the patches anyway :)