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

Do not finalize the value of the properties of a task that is skipped #7440

Open
adammurdoch opened this issue Oct 18, 2018 · 6 comments
Open
Assignees
Labels
a:feature A new functionality in:configuration-model lazy api, domain object container

Comments

@adammurdoch
Copy link
Member

#7371 added support to finalize the value of certain task input and output properties when the task starts execution. However, this finalization can be expensive and is pointless for many properties of a task that is skipped.

For example, finalizing the compile classpath for a compile task may involve a bunch of network access to resolve dependencies, however the value will be ignored if the task has no source files to compile and is skipped.

Expected Behavior

Finalize the value of only those properties of a task that are required to decide whether a task should be skipped. Only if the task will proceed should the remaining properties be finalized.

We should probably do the same with the "get from cache or run task actions" decision, and only finalize properties such as local state properties after we've decided to run the task actions.

The implementation might simply be to set all task properties to "finalize on next query" early in the execution lifecycle, so they are finalized only as required by whatever needs the value.

Current Behavior

Gradle does too much work when skipping tasks.

@adammurdoch adammurdoch added the in:configuration-model lazy api, domain object container label Oct 18, 2018
@lptr
Copy link
Member

lptr commented Oct 18, 2018

We already don't finalize inputs for tasks that are skipped early via onlyIf() or -x. I think this is only about primary inputs (@SkipWhenEmpty) that should be finalized first, checked if we can skip the task, and only then we should continue finalizing the rest of the inputs.

Is there another case we need to care about here?

@adammurdoch
Copy link
Member Author

I think we should just limit this issue to @SkipWhenEmpty properties.

Longer term, we should finalize and validate any properties needed to evaluate onlyIf(), then do the same for @SkipWhenEmpty properties if we need to (including any internal properties needed to calculate the values of these properties), then do the same for the properties we need for the cache key, then finally the properties we need to run the task actions.

@lptr
Copy link
Member

lptr commented Oct 19, 2018

That's a good point: we should always validate any property that has been finalized.

@adammurdoch
Copy link
Member Author

I want to look at task property validation as part of #4468, so that validation is triggered in some way by finalizing the value.

@stale
Copy link

stale bot commented Jul 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. If you're interested in how we try to keep the backlog in a healthy state, please read our blog post on how we refine our backlog. If you feel this is something you could contribute, please have a look at our Contributor Guide. Thank you for your contribution.

@stale stale bot added the stale label Jul 13, 2020
@stale
Copy link

stale bot commented Aug 3, 2020

This issue has been automatically closed due to inactivity. If you can reproduce this on a recent version of Gradle or if you have a good use case for this feature, please feel free to reopen the issue with steps to reproduce, a quick explanation of your use case or a high-quality pull request.

@stale stale bot closed this as completed Aug 3, 2020
@wolfs wolfs closed this as not planned Won't fix, can't repro, duplicate, stale Sep 16, 2022
@lptr lptr self-assigned this Sep 21, 2022
@lptr lptr added @execution and removed stale labels Sep 21, 2022
@wolfs wolfs reopened this Sep 22, 2022
@ov7a ov7a added the a:feature A new functionality label Sep 5, 2023
@lptr lptr removed the @execution label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:configuration-model lazy api, domain object container
Projects
None yet
Development

No branches or pull requests

4 participants