Skip to content

Commit

Permalink
Merge pull request #198 from gradle/no/disable-bundle-caching
Browse files Browse the repository at this point in the history
Disable caching of BundleLibraryClassesJar task.
  • Loading branch information
ghale authored Sep 15, 2021
2 parents 3594e5d + b9f94c9 commit 7876626
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 16 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,6 @@ room {
* There can only be a single schema export directory for the project - you cannot configure variant-specific export
directories. Schemas exported from different variants will be merged in the directory specified in the "room" extension.

### MergeNativeLibs, StripDebugSymbols, MergeJavaResources, and MergeSourceSetFolders Workarounds
### MergeNativeLibs, StripDebugSymbols, MergeJavaResources, MergeSourceSetFolders, and BundleLibraryClassesJar Workarounds

It has been observed that caching the `MergeNativeLibsTask`, `StripDebugSymbols`, `MergeJavaResources`, and `MergeSourceSetFolders` tasks rarely provide any significant positive avoidance savings. In fact, they frequently provide negative savings, especially when fetched from a remote cache node. As such, these workarounds actually disable caching for these tasks.
It has been observed that caching the `MergeNativeLibsTask`, `StripDebugSymbols`, `MergeJavaResources`, `MergeSourceSetFolders`, and `BundleLibraryClassesJar` tasks rarely provide any significant positive avoidance savings. In fact, they frequently provide negative savings, especially when fetched from a remote cache node. As such, these workarounds actually disable caching for these tasks.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package org.gradle.android

import com.google.common.collect.ImmutableList
import groovy.transform.CompileStatic
import org.gradle.android.workarounds.BundleLibraryClassesWorkaround
import org.gradle.android.workarounds.BundleLibraryClassesWorkaround_4_2
import org.gradle.android.workarounds.CompileLibraryResourcesWorkaround_4_0
import org.gradle.android.workarounds.CompileLibraryResourcesWorkaround_7_0
import org.gradle.android.workarounds.CompilerArgsProcessor
Expand Down Expand Up @@ -43,7 +45,9 @@ class AndroidCacheFixPlugin implements Plugin<Project> {
new CompileLibraryResourcesWorkaround_4_2(),
new CompileLibraryResourcesWorkaround_7_0(),
new MergeResourcesWorkaround(),
new StripDebugSymbolsWorkaround()
new StripDebugSymbolsWorkaround(),
new BundleLibraryClassesWorkaround(),
new BundleLibraryClassesWorkaround_4_2()
)
} else {
return Collections.emptyList()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.gradle.android.workarounds

import com.android.build.gradle.internal.tasks.BundleLibraryClasses
import org.gradle.android.AndroidIssue
import org.gradle.api.Project

/**
* Disables caching of the BundleLibraryClasses task which is mostly disk bound and unlikely to provide positive
* performance benefits.
*/
@AndroidIssue(introducedIn = "3.5.0", fixedIn = ["4.1.0-alpha01"], link = "https://issuetracker.google.com/issues/199763362")
class BundleLibraryClassesWorkaround implements Workaround {
private static final String CACHING_ENABLED_PROPERTY = "org.gradle.android.cache-fix.BundleLibraryClasses.caching.enabled"

@Override
void apply(WorkaroundContext context) {
context.project.tasks.withType(BundleLibraryClasses).configureEach {
outputs.doNotCacheIf("Caching BundleLibraryClasses is unlikely to provide positive performance results.", {true })
}
}

@Override
boolean canBeApplied(Project project) {
return !SystemPropertiesCompat.getBoolean(CACHING_ENABLED_PROPERTY, project)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.gradle.android.workarounds

import org.gradle.android.AndroidIssue
import org.gradle.api.Project

/**
* Disables caching of the BundleLibraryClassesJar and BundleLibraryClassesDir tasks which are mostly disk bound and
* unlikely to provide positive performance benefits.
*/
@AndroidIssue(introducedIn = "4.1.0", fixedIn = [], link = "https://issuetracker.google.com/issues/199763362")
class BundleLibraryClassesWorkaround_4_2 implements Workaround {
private static final String CACHING_ENABLED_PROPERTY = "org.gradle.android.cache-fix.BundleLibraryClasses.caching.enabled"

static Class<?> getJarTaskClass() {
return Class.forName('com.android.build.gradle.internal.tasks.BundleLibraryClassesJar')
}

static Class<?> getDirTaskClass() {
return Class.forName('com.android.build.gradle.internal.tasks.BundleLibraryClassesDir')
}


@Override
void apply(WorkaroundContext context) {
context.project.tasks.withType(jarTaskClass).configureEach {
outputs.doNotCacheIf("Caching BundleLibraryClassesJar is unlikely to provide positive performance results.", {true })
}
context.project.tasks.withType(dirTaskClass).configureEach {
outputs.doNotCacheIf("Caching BundleLibraryClassesDir is unlikely to provide positive performance results.", {true })
}
}

@Override
boolean canBeApplied(Project project) {
return !SystemPropertiesCompat.getBoolean(CACHING_ENABLED_PROPERTY, project)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -483,19 +483,19 @@ class CrossVersionOutcomeAndRelocationTest extends AbstractTest {
builder.expect(':library:mergeDebugGeneratedProguardFiles', FROM_CACHE)
builder.expect(':library:mergeReleaseConsumerProguardFiles', FROM_CACHE)
builder.expect(':library:mergeReleaseGeneratedProguardFiles', FROM_CACHE)
builder.expect(':library:bundleLibCompileToJarDebug', FROM_CACHE)
builder.expect(':library:bundleLibCompileToJarDebug', SUCCESS)
builder.expect(':library:bundleLibResDebug', NO_SOURCE)
builder.expect(':library:bundleLibResRelease', NO_SOURCE)
builder.expect(':library:bundleLibCompileToJarRelease', FROM_CACHE)
builder.expect(':library:bundleLibCompileToJarRelease', SUCCESS)
builder.expect(':library:stripDebugDebugSymbols', NO_SOURCE)
builder.expect(':library:stripReleaseDebugSymbols', NO_SOURCE)
builder.expect(':app:collectReleaseDependencies', SUCCESS)
builder.expect(':app:sdkReleaseDependencyData', SUCCESS)
}

static void android40xTo41xExpectation(ExpectedOutcomeBuilder builder) {
builder.expect(':library:bundleLibRuntimeToJarDebug', FROM_CACHE)
builder.expect(':library:bundleLibRuntimeToJarRelease', FROM_CACHE)
builder.expect(':library:bundleLibRuntimeToJarDebug', SUCCESS)
builder.expect(':library:bundleLibRuntimeToJarRelease', SUCCESS)
}

static void android41xOrHigherExpectations(ExpectedOutcomeBuilder builder) {
Expand Down Expand Up @@ -540,8 +540,8 @@ class CrossVersionOutcomeAndRelocationTest extends AbstractTest {

static void android42xOrHigherExpectations(ExpectedOutcomeBuilder builder) {
// Renamed from ToJar to ToDir
builder.expect(':library:bundleLibRuntimeToDirDebug', FROM_CACHE)
builder.expect(':library:bundleLibRuntimeToDirRelease', FROM_CACHE)
builder.expect(':library:bundleLibRuntimeToDirDebug', SUCCESS)
builder.expect(':library:bundleLibRuntimeToDirRelease', SUCCESS)
builder.expect(':app:optimizeReleaseResources', FROM_CACHE)
builder.expect(':app:mergeReleaseNativeDebugMetadata', NO_SOURCE)
builder.expect(':app:writeDebugAppMetadata', FROM_CACHE)
Expand Down
14 changes: 7 additions & 7 deletions src/test/groovy/org/gradle/android/WorkaroundTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ class WorkaroundTest extends Specification {
workarounds.collect { it.class.simpleName.replaceAll(/Workaround/, "") }.sort() == expectedWorkarounds.sort()
where:
androidVersion | expectedWorkarounds
"7.1.0-alpha11" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_7_0']
"7.0.2" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_7_0']
"4.2.2" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_2', 'MergeResources']
"4.1.3" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
"4.0.2" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
"3.6.4" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
"3.5.4" | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
"7.1.0-alpha11" | ['BundleLibraryClasses_4_2', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_7_0']
"7.0.2" | ['BundleLibraryClasses_4_2', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_7_0']
"4.2.2" | ['BundleLibraryClasses_4_2', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_2', 'MergeResources']
"4.1.3" | ['BundleLibraryClasses_4_2', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
"4.0.2" | ['BundleLibraryClasses', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
"3.6.4" | ['BundleLibraryClasses', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
"3.5.4" | ['BundleLibraryClasses', 'MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
}
}

0 comments on commit 7876626

Please sign in to comment.