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

Slow UP-TO-DATE checks if filtered FileTree inputs are used #8559

Open
hungvietnguyen opened this issue Feb 20, 2019 · 5 comments

Comments

@hungvietnguyen
Copy link

commented Feb 20, 2019

Current Behavior

In the Android Gradle plugin, we have observed that when we used a filtered FileTree as a task's input, the UP-TO-DATE check for that task would take a long time. I'm not sure whether the "filtered" part or the "FileTree" part makes it slow, or both.

Example input in AidlCompile (reported by an AGP user as a side issue at https://issuetracker.google.com/121251997):

private static final PatternSet PATTERN_SET = new PatternSet().include("**/*.aidl");

@InputFiles
@SkipWhenEmpty
@PathSensitive(PathSensitivity.RELATIVE)
public FileTree getSourceFiles() {
    return getProject().files(sourceDirs.get()).getAsFileTree().matching(PATTERN_SET);
}

Example input in DexMergingTask (tried internally then reverted due to performance regression):

@InputFiles
@PathSensitive(PathSensitivity.NONE)
fun getDexFilesForInput(): FileCollection =
    dexFiles.asFileTree.filter { it.name != "__content__.json" }

Request

I'm filing this bug to check if the slowness is expected, and whether something needs to be fixed in Gradle or the way we use filtered FileTree to reduce this overhead.

@wolfs

This comment has been minimized.

Copy link
Member

commented Feb 20, 2019

The slowdown is probably due to Gradle not caching the file system state for filtered file trees. So every time the input is checked for changes, the whole file tree needs to be inspected again.

If there is no pattern, then the whole file tree is cached in memory and returned next time it is queried.

Are the .aidl files at the top-level of the src directory only? Then it is probably faster to use .include("*.aidl") - no need to inspect all the sub-directories.

For the dex files, it think that .filter behaves even worse than .include - so .exclude("__content__.json") or .exclude("**/__content__.json") should be faster. It would still be slower that without the pattern, due to the missing in memory caching.

@hungvietnguyen

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

Thanks for your explanation!

.aidl files are similar to .java files in terms of directory structure, so we have to use **/*.aidl.

That's a good tip regarding include/exclude vs. filter (although I find it surprising that regex matching is faster than string comparison).

@scott-pollom

This comment has been minimized.

Copy link

commented Mar 2, 2019

Is there any plan for Gradle to cache the file system state for filtered file trees?

@wolfs

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

We currently don't have something planned.

@vanniktech

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

@devisnik and I talked at Droidcon Berlin and we were able to reproduce this in one of my projects when using the following fileTree as an input:

fun Project.editorconfigFiles() = fileTree(mapOf("dir" to ".", "include" to "**/.editorconfig"))

Reference to the open source plugin:

https://github.com/vanniktech/gradle-code-quality-tools-plugin/blob/0f9ba2ea6f8eebb53c7498988acce5bea743915c/src/main/kotlin/com/vanniktech/code/quality/tools/CodeQualityToolsPlugin.kt#L84

Sample run on my Macbook with my project:

BUILD SUCCESSFUL in 1m 47s
56 actionable tasks: 56 up-to-date

Publishing a build scan to scans.gradle.com requires accepting the Gradle Terms of Service defined at https://gradle.com/terms-of-service. Do you accept these terms? [yes, no] yes

Gradle Terms of Service accepted.

Publishing build scan...
https://gradle.com/s/pswsy4epczylq

Without the editorconfigFiles input I'm getting a successful 56 up-to-date build within 5-6 seconds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.