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

Allow copy-file and delete-file to be scheduled #574

Merged
merged 1 commit into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ A scenario can define changes that should be applied to the source before each b
- `clear-jars-cache-before`: Deletes the contents of the instrumented jars cache before the scenario is executed (`SCENARIO`), before cleanup (`CLEANUP`) or before the build is executed (`BUILD`).
- `clear-project-cache-before`: Deletes the contents of the `.gradle` and `buildSrc/.gradle` project cache directories before the scenario is executed (`SCENARIO`), before cleanup (`CLEANUP`) or before the build is executed (`BUILD`).
- `clear-transform-cache-before`: Deletes the contents of the transform cache before the scenario is executed (`SCENARIO`), before cleanup (`CLEANUP`) or before the build is executed (`BUILD`).
- `copy-file`: Copies a file or a directory from one location to another. Has to specify a `source` and a `target` path; relative paths are resolved against the project directory. Can take an array of operations.
- `delete-file`: Deletes a file or a directory. Has to specify a `target` path; when relative it is resolved against the project directory. Can take an array of operations.
- `copy-file`: Copies a file or a directory from one location to another. Has to specify a `source` and a `target` path; relative paths are resolved against the project directory. Can also specify a schedule (defaults `SCENARIO`). Can take an array of operations.
- `delete-file`: Deletes a file or a directory. Has to specify a `target` path; when relative it is resolved against the project directory. Can also specify a schedule (defaults `SCENARIO`). Can take an array of operations.
- `git-checkout`: Checks out a specific commit for the build step, and a different one for the cleanup step.
- `git-revert`: Reverts a given set of commits before the build and resets it afterward.
- `iterations`: Number of builds to actually measure
Expand Down Expand Up @@ -352,8 +352,10 @@ They can be added to a scenario file like this:
}
delete-file = [{
target = ".mvn/develocity.xml"
schedule = CLEANUP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Nice

}, {
target = ".gradle"
schedule = CLEANUP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ This then runs at the start of schedule, right?

Can we maybe add this detail also do README.md that it means at start/before a schedule, or should that be obvious?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I've extracted some words about scheduling mutators.

}]
git-checkout = {
cleanup = "efb43a1"
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/gradle/profiler/BuildInvoker.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public int profileWarmUps() {
return 2;
}

public boolean allowsCleanupBetweenBuilds() {
public boolean allowsMutationBetweenBuilds() {
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/gradle/profiler/ScenarioLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import org.gradle.profiler.buck.BuckScenarioDefinition;
import org.gradle.profiler.gradle.*;
import org.gradle.profiler.maven.MavenScenarioDefinition;
import org.gradle.profiler.mutations.AbstractCleanupMutator.CleanupSchedule;
import org.gradle.profiler.mutations.AbstractScheduledMutator.Schedule;
import org.gradle.profiler.mutations.ApplyAbiChangeToSourceFileMutator;
import org.gradle.profiler.mutations.ApplyBuildScriptChangeFileMutator;
import org.gradle.profiler.mutations.ApplyChangeToAndroidLayoutFileMutator;
Expand Down Expand Up @@ -444,7 +444,7 @@ public static GradleBuildInvoker invoker(Config config, GradleBuildInvoker defau
}

private static GradleBuildInvoker getAndroidStudioInvoker(Config config) {
CleanupSchedule schedule = ConfigUtil.enumValue(config, CLEAR_ANDROID_STUDIO_CACHE_BEFORE, CleanupSchedule.class, null);
Schedule schedule = ConfigUtil.enumValue(config, CLEAR_ANDROID_STUDIO_CACHE_BEFORE, Schedule.class, null);
if (schedule == null) {
return GradleBuildInvoker.AndroidStudio;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ private GradleBuildInvoker(GradleClientSpec client, GradleDaemonReuse daemonReus
}

@Override
public boolean allowsCleanupBetweenBuilds() {
public boolean allowsMutationBetweenBuilds() {
// Warm daemons keep caches open and thus they cannot be removed
return !isReuseDaemon();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@

import java.io.File;

public abstract class AbstractCacheCleanupMutator extends AbstractCleanupMutator {
public abstract class AbstractCacheCleanupMutator extends AbstractScheduledMutator {

private final File gradleUserHome;
private final String cacheNamePrefix;

public AbstractCacheCleanupMutator(File gradleUserHome, CleanupSchedule schedule, String cacheNamePrefix) {
public AbstractCacheCleanupMutator(File gradleUserHome, Schedule schedule, String cacheNamePrefix) {
super(schedule);
this.gradleUserHome = gradleUserHome;
this.cacheNamePrefix = cacheNamePrefix;
}

@Override
protected void cleanup() {
protected void executeOnSchedule() {
System.out.println("> Cleaning " + cacheNamePrefix + " caches in " + gradleUserHome);
File cachesDir = new File(gradleUserHome, "caches");
if (cachesDir.isDirectory()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package org.gradle.profiler.mutations;

import org.gradle.profiler.BuildMutator;

import java.io.File;

public class AbstractFileSystemMutator implements BuildMutator {
public abstract class AbstractFileSystemMutator extends AbstractScheduledMutator {

protected AbstractFileSystemMutator(Schedule schedule) {
super(schedule);
}

protected static File resolveProjectFile(File projectDir, String path) {
File file = new File(path);
if (file.isAbsolute()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,43 +11,43 @@
import java.io.IOException;
import java.nio.file.Files;

public abstract class AbstractCleanupMutator implements BuildMutator {
public abstract class AbstractScheduledMutator implements BuildMutator {

private final CleanupSchedule schedule;
private final Schedule schedule;

public AbstractCleanupMutator(CleanupSchedule schedule) {
public AbstractScheduledMutator(Schedule schedule) {
this.schedule = schedule;
}

@Override
public void validate(BuildInvoker invoker) {
if (schedule != CleanupSchedule.SCENARIO && !invoker.allowsCleanupBetweenBuilds()) {
if (schedule != Schedule.SCENARIO && !invoker.allowsMutationBetweenBuilds()) {
throw new IllegalStateException(this + " is not allowed to be executed between builds with invoker " + invoker);
}
}

@Override
public void beforeBuild(BuildContext context) {
if (schedule == CleanupSchedule.BUILD) {
cleanup();
if (schedule == Schedule.BUILD) {
executeOnSchedule();
}
}

@Override
public void beforeScenario(ScenarioContext context) {
if (schedule == CleanupSchedule.SCENARIO) {
cleanup();
if (schedule == Schedule.SCENARIO) {
executeOnSchedule();
}
}

@Override
public void beforeCleanup(BuildContext context) {
if (schedule == CleanupSchedule.CLEANUP) {
cleanup();
if (schedule == Schedule.CLEANUP) {
executeOnSchedule();
}
}

abstract protected void cleanup();
abstract protected void executeOnSchedule();

protected static void delete(File f) {
try {
Expand All @@ -64,22 +64,22 @@ protected static void delete(File f) {
protected static abstract class Configurator implements BuildMutatorConfigurator {
@Override
public BuildMutator configure(String key, BuildMutatorConfiguratorSpec spec) {
CleanupSchedule schedule = ConfigUtil.enumValue(spec.getScenario(), key, CleanupSchedule.class, null);
Schedule schedule = ConfigUtil.enumValue(spec.getScenario(), key, Schedule.class, null);
if (schedule == null) {
throw new IllegalArgumentException("Schedule for cleanup is not specified");
throw new IllegalArgumentException("Schedule is not specified");
}
return newInstance(spec, key, schedule);
}

protected abstract BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule);
protected abstract BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule);
}

@Override
public String toString() {
return getClass().getSimpleName() + "(" + schedule + ")";
}

public enum CleanupSchedule {
public enum Schedule {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Also this change will require some changes in gradle/gradle since we use these constants in few places, e.g.:
https://github.com/gradle/gradle/blob/381460e0f3dca2bb39d77d91e474f390a9ef812b/testing/performance/src/performanceTest/groovy/org/gradle/performance/experiment/declarativedsl/DeclarativeDslFirstUsePerformanceTest.groovy#L49-L54

It's not a problem but we should be aware of that when we upgrade a profiler.

SCENARIO, CLEANUP, BUILD
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

public class ClearArtifactTransformCacheMutator extends AbstractCacheCleanupMutator {

public ClearArtifactTransformCacheMutator(File gradleUserHome, CleanupSchedule schedule) {
public ClearArtifactTransformCacheMutator(File gradleUserHome, Schedule schedule) {
super(gradleUserHome, schedule, "transforms-");
}

Expand All @@ -15,9 +15,9 @@ protected void cleanupCacheDir(File cacheDir) {
delete(cacheDir);
}

public static class Configurator extends AbstractCleanupMutator.Configurator {
public static class Configurator extends AbstractScheduledMutator.Configurator {
@Override
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule) {
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule) {
return new ClearArtifactTransformCacheMutator(spec.getGradleUserHome(), schedule);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,23 @@

public class ClearBuildCacheMutator extends AbstractCacheCleanupMutator {

public ClearBuildCacheMutator(File gradleUserHome, CleanupSchedule schedule) {
public ClearBuildCacheMutator(File gradleUserHome, Schedule schedule) {
super(gradleUserHome, schedule, "build-cache-");
}

@Override
protected void cleanupCacheDir(File cacheDir) {
Arrays.stream(Objects.requireNonNull(cacheDir.listFiles(ClearBuildCacheMutator::shouldRemoveFile))).forEach(AbstractCleanupMutator::delete);
Arrays.stream(Objects.requireNonNull(cacheDir.listFiles(ClearBuildCacheMutator::shouldRemoveFile))).forEach(AbstractScheduledMutator::delete);
}

private static boolean shouldRemoveFile(File file) {
// First generation Build-cache contains hashes of length 32 as names, second generation uses a H2 database
return file.getName().length() == 32 || file.getName().endsWith(".db");
}

public static class Configurator extends AbstractCleanupMutator.Configurator {
public static class Configurator extends AbstractScheduledMutator.Configurator {
@Override
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule) {
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule) {
return new ClearBuildCacheMutator(spec.getGradleUserHome(), schedule);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
import java.io.IOException;
import java.io.UncheckedIOException;

public class ClearConfigurationCacheStateMutator extends AbstractCleanupMutator {
public class ClearConfigurationCacheStateMutator extends AbstractScheduledMutator {
private final File projectDir;

public ClearConfigurationCacheStateMutator(File projectDir, CleanupSchedule schedule) {
public ClearConfigurationCacheStateMutator(File projectDir, Schedule schedule) {
super(schedule);
this.projectDir = projectDir;
}
Expand All @@ -22,7 +22,7 @@ public void validate(BuildInvoker invoker) {
}

@Override
protected void cleanup() {
protected void executeOnSchedule() {
System.out.println("> Cleaning configuration cache state");
cleanup(new File(projectDir, ".gradle/configuration-cache"));
cleanup(new File(projectDir, ".instant-execution-state"));
Expand All @@ -36,9 +36,9 @@ private void cleanup(File dir) {
}
}

public static class Configurator extends AbstractCleanupMutator.Configurator {
public static class Configurator extends AbstractScheduledMutator.Configurator {
@Override
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule) {
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule) {
return new ClearConfigurationCacheStateMutator(spec.getProjectDir(), schedule);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
import java.io.UncheckedIOException;
import java.nio.file.Files;

public class ClearGradleUserHomeMutator extends AbstractCleanupMutator {
public class ClearGradleUserHomeMutator extends AbstractScheduledMutator {
private final File gradleUserHome;

public ClearGradleUserHomeMutator(File gradleUserHome, CleanupSchedule schedule) {
public ClearGradleUserHomeMutator(File gradleUserHome, Schedule schedule) {
super(schedule);
this.gradleUserHome = gradleUserHome;
}

@Override
protected void cleanup() {
protected void executeOnSchedule() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Note: This change will require also a change of some logic in gradle/gradle when we upgrade the profiler. I know about this one:
https://github.com/gradle/gradle/blob/9b00b5d6f111a3542959598fb31c5c7181c15972/testing/internal-performance-testing/src/main/groovy/org/gradle/performance/mutator/ClearArtifactTransformCacheWithoutInstrumentedJarsMutator.groovy#L45

It's not a problem but we should be aware of that when we upgrade a profiler.

System.out.println(String.format("> Cleaning Gradle user home: %s", gradleUserHome.getAbsolutePath()));
if (!gradleUserHome.exists()) {
throw new IllegalArgumentException(String.format(
Expand All @@ -34,9 +34,9 @@ protected void cleanup() {
}
}

public static class Configurator extends AbstractCleanupMutator.Configurator {
public static class Configurator extends AbstractScheduledMutator.Configurator {
@Override
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule) {
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule) {
return new ClearGradleUserHomeMutator(spec.getGradleUserHome(), schedule);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

public class ClearJarsCacheMutator extends AbstractCacheCleanupMutator {

public ClearJarsCacheMutator(File gradleUserHome, CleanupSchedule schedule) {
public ClearJarsCacheMutator(File gradleUserHome, Schedule schedule) {
super(gradleUserHome, schedule, "jars-");
}

Expand All @@ -15,9 +15,9 @@ protected void cleanupCacheDir(File cacheDir) {
delete(cacheDir);
}

public static class Configurator extends AbstractCleanupMutator.Configurator {
public static class Configurator extends AbstractScheduledMutator.Configurator {
@Override
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule) {
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule) {
return new ClearJarsCacheMutator(spec.getGradleUserHome(), schedule);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
import java.io.IOException;
import java.io.UncheckedIOException;

public class ClearProjectCacheMutator extends AbstractCleanupMutator {
public class ClearProjectCacheMutator extends AbstractScheduledMutator {
private final File projectDir;

public ClearProjectCacheMutator(File projectDir, CleanupSchedule schedule) {
public ClearProjectCacheMutator(File projectDir, Schedule schedule) {
super(schedule);
this.projectDir = projectDir;
}

@Override
protected void cleanup() {
protected void executeOnSchedule() {
deleteGradleCache("project", projectDir);
File buildSrc = new File(projectDir, "buildSrc");
if (buildSrc.exists()) {
Expand All @@ -34,9 +34,9 @@ private void deleteGradleCache(String name, File baseDir) {
}
}

public static class Configurator extends AbstractCleanupMutator.Configurator {
public static class Configurator extends AbstractScheduledMutator.Configurator {
@Override
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, CleanupSchedule schedule) {
protected BuildMutator newInstance(BuildMutatorConfiguratorSpec spec, String key, Schedule schedule) {
return new ClearProjectCacheMutator(spec.getProjectDir(), schedule);
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/gradle/profiler/mutations/CopyFileMutator.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import org.gradle.profiler.BuildMutator;
import org.gradle.profiler.CompositeBuildMutator;
import org.gradle.profiler.ConfigUtil;
import org.gradle.profiler.ScenarioContext;

import java.io.File;
import java.io.IOException;
Expand All @@ -18,14 +17,14 @@ public class CopyFileMutator extends AbstractFileSystemMutator {
private final File source;
private final File target;

public CopyFileMutator(File source, File target) {
public CopyFileMutator(File source, File target, Schedule schedule) {
super(schedule);
this.source = source;
this.target = target;
}

@SuppressWarnings("UnstableApiUsage")
@Override
public void beforeScenario(ScenarioContext context) {
protected void executeOnSchedule() {
try {
System.out.println("Copying '" + source.getAbsolutePath() + "' to '" + target.getAbsolutePath() + "'");
if (!target.exists()) {
Expand Down Expand Up @@ -54,12 +53,13 @@ public BuildMutator configure(String key, BuildMutatorConfiguratorSpec spec) {
private static CopyFileMutator createMutator(BuildMutatorConfiguratorSpec spec, Config config) {
String source = ConfigUtil.string(config, "source", null);
String target = ConfigUtil.string(config, "target", null);
Schedule schedule = ConfigUtil.enumValue(config, "schedule", Schedule.class, Schedule.SCENARIO);

if (Strings.isNullOrEmpty(source) || Strings.isNullOrEmpty(target)) {
throw new IllegalArgumentException("The `source` and `target` are required for copy-file");
}
File projectDir = spec.getProjectDir();
return new CopyFileMutator(resolveProjectFile(projectDir, source), resolveProjectFile(projectDir, target));
return new CopyFileMutator(resolveProjectFile(projectDir, source), resolveProjectFile(projectDir, target), schedule);
}
}
}
Loading
Loading