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

[2.x] Add Def.cacheTaskWithUpdate to update the task output after a cache reuse #7559

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

adpi2
Copy link
Member

@adpi2 adpi2 commented May 15, 2024

After a cache hit of the incremental compilation we need to:

  • Unzip the jar to the backend output folder to enable incremental compilation.
  • Update the timestamps. Those timestamps are used by the testQuickFilter to decide which class needs to be rerun.

To do so I introduced:

Def.cachedTaskWithUpdate[A1: JsonFormat](inline a1: A1)(inline onCacheReuse: (A1 => A1)): Def.Initialize[Task[A1]]

The onCacheReuse is only called after a cache hit to update the output. The user can declare any task dependency in onCacheReuse with .value.

Example:

val foo = Def.task(1)
val bar = Def.task(2)
val fizz = Def.cachedTaskWithUpdate(foo.value)(res => res + bar.value)

The expansion of fizz looks like this:

val fizz = (foo, bar).mapN { (foo, bar) => // both foo and bar are dependencies of the task
  Def.cachedTask(input = foo) {            // only foo is used as an input
    action: _ => foo
    onCacheReuse: () => res => res + bar
  }
}

The behavior of fizz is:

sbt:example> show fizz
1 // computed and cached
sbt:example> show fizz
3 // taken from the cache and updated (1 + 2) 
sbt:example> show fizz
3 // taken from the cache and updated (1 + 2) 

@adpi2 adpi2 changed the title [2.x] Add Def.cacheTaskWithUpdate to perform an update of the output after a cache reuse [2.x] Add Def.cacheTaskWithUpdate to update the task output after a cache reuse May 16, 2024
@adpi2 adpi2 requested a review from eed3si9n May 16, 2024 14:47
@adpi2 adpi2 marked this pull request as ready for review May 16, 2024 14:47
@eed3si9n
Copy link
Member

After a cache hit of the incremental compilation we need to:

  • Unzip the jar to the backend output folder to enable incremental compilation.
  • Update the timestamps. Those timestamps are used by the testQuickFilter to decide which class needs to be rerun.

If possible, I feel like we should try to fix the incremental compilation and testQuick so they can work with the remote downloaded outputs instead. Even if we're introducing some side effects, I feel like it should be narrowly-scoped (maybe it should be Seq[Path] => Unit), and should always run to avoid potentially confusing results if misused/abused.

@adpi2
Copy link
Member Author

adpi2 commented May 21, 2024

If possible, I feel like we should try to fix the incremental compilation and testQuick so they can work with the remote downloaded outputs instead.

It would be ideal. But also it means we cannot rely on timestamps in CompileAnalysis anymore, which could potentially break some plugins/projects. It's probably very limited though.

For incremental compilation, I think we can configure Zinc to always compile to a jar file. But I don't know how good that works, or if it has already been battle-tested.

For testQuick, if we cannot use the timestamps, we can still use the hashes. But that mean we need to compute the hash of each test even after a test which is a bit of overhead.

Even if we're introducing some side effects, I feel like it should be narrowly-scoped (maybe it should be Seq[Path] => Unit), and should always run to avoid potentially confusing results if misused/abused.

Yeah, I thought of that too and Seq[Path] => Unit is probably harder to abuse. I also wonder if Unit would not be even better (no input and pure side effect). Those Seq[Path] are opaque and it would be dangerous to guess which path is which output file. We can still use .value to find the file that we need to update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants