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

Collect numeric properties of build operations #261

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 7 additions & 14 deletions src/main/java/org/gradle/profiler/GradleBuildInvocationResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.gradle.profiler.result.BuildInvocationResult;
import org.gradle.profiler.result.Sample;
import org.gradle.profiler.result.TimeSample;

import javax.annotation.Nullable;
import java.time.Duration;
Expand All @@ -13,27 +14,28 @@ public class GradleBuildInvocationResult extends BuildInvocationResult {
private final Duration timeToTaskExecution;
private final Map<String, Duration> cumulativeBuildOperationTimes;
private final String daemonPid;
public static final Sample<GradleBuildInvocationResult> TIME_TO_TASK_EXECUTION = new Sample<GradleBuildInvocationResult>() {

public static final Sample<GradleBuildInvocationResult> TIME_TO_TASK_EXECUTION = new TimeSample<GradleBuildInvocationResult>() {
@Override
public String getName() {
return "task start";
}

@Override
public Duration extractFrom(GradleBuildInvocationResult result) {
protected Duration getDuration(GradleBuildInvocationResult result) {
return result.timeToTaskExecution;
}
};

public static Sample<GradleBuildInvocationResult> sampleBuildOperation(String buildOperationDetailsClass) {
return new Sample<GradleBuildInvocationResult>() {
public static Sample<GradleBuildInvocationResult> sampleBuildOperationDuration(String buildOperationDetailsClass) {
return new TimeSample<GradleBuildInvocationResult>() {
@Override
public String getName() {
return getSimpleBuildOperationName(buildOperationDetailsClass);
}

@Override
public Duration extractFrom(GradleBuildInvocationResult result) {
protected Duration getDuration(GradleBuildInvocationResult result) {
return result.cumulativeBuildOperationTimes.getOrDefault(buildOperationDetailsClass, Duration.ZERO);
}
};
Expand All @@ -49,13 +51,4 @@ public GradleBuildInvocationResult(BuildContext buildContext, Duration execution
public String getDaemonPid() {
return daemonPid;
}

@Nullable
public Duration getTimeToTaskExecution() {
return timeToTaskExecution;
}

public Map<String, Duration> getCumulativeBuildOperationTimes() {
return cumulativeBuildOperationTimes;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public List<Sample<? super GradleBuildInvocationResult>> samplesFor(InvocationSe
builder.add(GradleBuildInvocationResult.TIME_TO_TASK_EXECUTION);
}
scenario.getMeasuredBuildOperations().stream()
.map(GradleBuildInvocationResult::sampleBuildOperation)
.map(GradleBuildInvocationResult::sampleBuildOperationDuration)
.forEach(builder::add);
return builder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public Map<String, Duration> getCumulativeBuildOperationTimes() {
private static Duration readCumulativeTimeFromDataFile(File dataFile) {
try {
try (Stream<String> lines = Files.lines(dataFile.toPath(), StandardCharsets.UTF_8)) {
return Duration.ofMillis(lines.mapToLong(Long::parseLong).sum());
return Duration.ofNanos((long) lines.mapToDouble(Double::parseDouble).sum() * 1000000);
}
} catch (IOException e) {
throw new RuntimeException("Could not read result from file.", e);
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/org/gradle/profiler/report/CsvGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.io.BufferedWriter;
import java.io.File;
import java.io.IOException;
import java.time.Duration;
import java.util.List;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -110,7 +109,6 @@ private <T extends BuildInvocationResult> void writeWideRow(BufferedWriter write
T buildResult = results.get(row);
writer.write(scenario.getSamples().stream()
.map(sample -> sample.extractFrom(buildResult))
.map(Duration::toMillis)
.map(Object::toString)
.collect(Collectors.joining(","))
);
Expand Down Expand Up @@ -139,7 +137,7 @@ private <T extends BuildInvocationResult> void writeLongRow(BufferedWriter write
writer.write(",");
writer.write(sample.getName());
writer.write(",");
writer.write(String.valueOf(sample.extractFrom(result).toMillis()));
writer.write(String.valueOf(sample.extractFrom(result)));
writer.newLine();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import javax.annotation.OverridingMethodsMustInvokeSuper;
import java.io.Writer;
import java.lang.reflect.Type;
import java.time.Duration;
import java.time.format.DateTimeFormatter;
import java.time.temporal.Temporal;
import java.util.List;
Expand Down Expand Up @@ -97,6 +96,7 @@ private <T extends BuildInvocationResult> JsonObject serializeScenarioResult(Bui
private JsonObject serializeSample(BuildScenarioResult<? extends BuildInvocationResult> scenarioResult, Sample<?> sample) {
JsonObject json = new JsonObject();
json.addProperty("name", sample.getName());
json.addProperty("unit", sample.getUnit());
return json;
}

Expand All @@ -108,8 +108,8 @@ private <T extends BuildInvocationResult> JsonObject serializeIteration(T result
json.addProperty("title", result.getBuildContext().getDisplayName());
JsonObject valuesJson = new JsonObject();
for (Sample<? super T> sample : samples) {
Duration value = sample.extractFrom(result);
valuesJson.addProperty(sample.getName(), value.toNanos() / 1000000d);
double value = sample.extractFrom(result);
valuesJson.addProperty(sample.getName(), value);
}
json.add("values", valuesJson);
return json;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ public class BuildInvocationResult {
private final BuildContext buildContext;
private final Duration executionTime;

public static final Sample<BuildInvocationResult> EXECUTION_TIME = new Sample<BuildInvocationResult>() {
public static final Sample<BuildInvocationResult> EXECUTION_TIME = new TimeSample<BuildInvocationResult>() {
@Override
public String getName() {
return "execution";
}

@Override
public Duration extractFrom(BuildInvocationResult result) {
protected Duration getDuration(BuildInvocationResult result) {
return result.getExecutionTime();
}
};
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/gradle/profiler/result/Sample.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.gradle.profiler.result;

import java.time.Duration;

public interface Sample<T extends BuildInvocationResult> {
String getName();

Duration extractFrom(T result);
String getUnit();

double extractFrom(T result);
}
18 changes: 18 additions & 0 deletions src/main/java/org/gradle/profiler/result/TimeSample.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.gradle.profiler.result;

import java.time.Duration;

public abstract class TimeSample<T extends BuildInvocationResult> implements Sample<T> {

@Override
public String getUnit() {
return "ms";
}

@Override
public double extractFrom(T result) {
return getDuration(result).toNanos() / 1000000d;
}

abstract protected Duration getDuration(T result);
}
1 change: 0 additions & 1 deletion src/main/js/org/gradle/profiler/report/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ new Vue({
sample.color = `hsl(${scenarioIndex * 360 / scenarios.length}, ${100 - 80 * sampleIndex / samples.length}%, ${30 + 40 * sampleIndex / samples.length}%)`;
sample.thickness = sampleIndex === 0 ? 3 : 2;
sample.selected = sampleIndex === 0;
sample.unit = "ms";
const data = measuredIterations(scenario).map(iteration => iteration.values[sample.name]);
OPERATIONS.forEach(operation => sample[operation.name] = operation.apply(data));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ genrule(
assert lines.get(1) == "version,Gradle ${gradleVersion}"
assert lines.get(2) == "tasks,${tasks.join(", ")}"
assert lines.get(3) == "value,execution"
assert lines.get(4).matches("warm-up build #1,\\d+")
assert lines.get(9).matches("warm-up build #6,\\d+")
assert lines.get(10).matches("measured build #1,\\d+")
assert lines.get(11).matches("measured build #2,\\d+")
assert lines.get(19).matches("measured build #10,\\d+")
assert lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?/)
assert lines.get(9).matches(/warm-up build #6,\d+(?:\.\d+)?/)
assert lines.get(10).matches(/measured build #1,\d+(?:\.\d+)?/)
assert lines.get(11).matches(/measured build #2,\d+(?:\.\d+)?/)
assert lines.get(19).matches(/measured build #10,\d+(?:\.\d+)?/)
}

/**
Expand All @@ -176,10 +176,10 @@ genrule(
assert lines.get(1) == "version,Gradle ${gradleVersion}"
assert lines.get(2) == "tasks,${tasks.join(", ")}"
assert lines.get(3) == "value,execution"
assert lines.get(4).matches("warm-up build #1,\\d+")
assert lines.get(5).matches("measured build #1,\\d+")
assert lines.get(6).matches("measured build #2,\\d+")
assert lines.get(14).matches("measured build #10,\\d+")
assert lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?/)
assert lines.get(5).matches(/measured build #1,\d+(?:\.\d+)?/)
assert lines.get(6).matches(/measured build #2,\d+(?:\.\d+)?/)
assert lines.get(14).matches(/measured build #10,\d+(?:\.\d+)?/)
}

/**
Expand All @@ -192,10 +192,10 @@ genrule(
assert lines.get(1) == "version,Gradle ${gradleVersion}"
assert lines.get(2) == "tasks,${tasks.join(", ")}"
assert lines.get(3) == "value,execution"
assert lines.get(4).matches("warm-up build #1,\\d+")
assert lines.get(5).matches("measured build #1,\\d+")
assert lines.get(6).matches("measured build #2,\\d+")
assert lines.get(14).matches("measured build #10,\\d+")
assert lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?/)
assert lines.get(5).matches(/measured build #1,\d+(?:\.\d+)?/)
assert lines.get(6).matches(/measured build #2,\d+(?:\.\d+)?/)
assert lines.get(14).matches(/measured build #10,\d+(?:\.\d+)?/)
}
}

Expand Down
42 changes: 21 additions & 21 deletions src/test/groovy/org/gradle/profiler/BenchmarkIntegrationTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ class BenchmarkIntegrationTest extends AbstractProfilerIntegrationTest {
lines.get(1) == "version,Gradle ${minimalSupportedGradleVersion},Gradle 5.0"
lines.get(2) == "tasks,assemble,assemble"
lines.get(3) == "value,execution,execution"
lines.get(4).matches("warm-up build #1,\\d+,\\d+")
lines.get(9).matches("warm-up build #6,\\d+,\\d+")
lines.get(10).matches("measured build #1,\\d+,\\d+")
lines.get(11).matches("measured build #2,\\d+,\\d+")
lines.get(12).matches("measured build #3,,\\d+")
lines.get(19).matches("measured build #10,,\\d+")
lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(9).matches(/warm-up build #6,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(10).matches(/measured build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(11).matches(/measured build #2,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(12).matches(/measured build #3,,\d+(?:\.\d+)?/)
lines.get(19).matches(/measured build #10,,\d+(?:\.\d+)?/)
}

def "recovers from failure in warm-up build while running benchmarks"() {
Expand Down Expand Up @@ -62,15 +62,15 @@ class BenchmarkIntegrationTest extends AbstractProfilerIntegrationTest {
lines.get(1) == "version,Gradle ${minimalSupportedGradleVersion},Gradle 5.0"
lines.get(2) == "tasks,assemble,assemble"
lines.get(3) == "value,execution,execution"
lines.get(4).matches("warm-up build #1,\\d+,\\d+")
lines.get(5).matches("warm-up build #2,\\d+,\\d+")
lines.get(6).matches("warm-up build #3,\\d+,\\d+")
lines.get(7).matches("warm-up build #4,,\\d+")
lines.get(8).matches("warm-up build #5,,\\d+")
lines.get(9).matches("warm-up build #6,,\\d+")
lines.get(10).matches("measured build #1,,\\d+")
lines.get(11).matches("measured build #2,,\\d+")
lines.get(19).matches("measured build #10,,\\d+")
lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(5).matches(/warm-up build #2,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(6).matches(/warm-up build #3,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(7).matches(/warm-up build #4,,\d+(?:\.\d+)?/)
lines.get(8).matches(/warm-up build #5,,\d+(?:\.\d+)?/)
lines.get(9).matches(/warm-up build #6,,\d+(?:\.\d+)?/)
lines.get(10).matches(/measured build #1,,\d+(?:\.\d+)?/)
lines.get(11).matches(/measured build #2,,\d+(?:\.\d+)?/)
lines.get(19).matches(/measured build #10,,\d+(?:\.\d+)?/)
}

def "recovers from failure to run any builds while running benchmarks"() {
Expand Down Expand Up @@ -99,12 +99,12 @@ class BenchmarkIntegrationTest extends AbstractProfilerIntegrationTest {
lines.get(1) == "version,Gradle ${minimalSupportedGradleVersion},Gradle 5.0"
lines.get(2) == "tasks,assemble,assemble"
lines.get(3) == "value,execution,execution"
lines.get(4).matches("warm-up build #1,,\\d+")
lines.get(5).matches("warm-up build #2,,\\d+")
lines.get(6).matches("warm-up build #3,,\\d+")
lines.get(10).matches("measured build #1,,\\d+")
lines.get(11).matches("measured build #2,,\\d+")
lines.get(19).matches("measured build #10,,\\d+")
lines.get(4).matches(/warm-up build #1,,\d+(?:\.\d+)?/)
lines.get(5).matches(/warm-up build #2,,\d+(?:\.\d+)?/)
lines.get(6).matches(/warm-up build #3,,\d+(?:\.\d+)?/)
lines.get(10).matches(/measured build #1,,\d+(?:\.\d+)?/)
lines.get(11).matches(/measured build #2,,\d+(?:\.\d+)?/)
lines.get(19).matches(/measured build #10,,\d+(?:\.\d+)?/)
}

def brokenBuild(int successfulIterations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ class BuildOperationInstrumentationIntegrationTest extends AbstractProfilerInteg
lines.get(1) == "version,Gradle ${gradleVersion},Gradle ${gradleVersion}"
lines.get(2) == "tasks,assemble,assemble"
lines.get(3) == "value,execution,task start"
lines.get(4).matches("warm-up build #1,\\d+,\\d+")
lines.get(9).matches("warm-up build #6,\\d+,\\d+")
lines.get(10).matches("measured build #1,\\d+,\\d+")
lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(9).matches(/warm-up build #6,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(10).matches(/measured build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)

and:
def taskStart = lines.get(10) =~ /measured build #1,\d+,(\d+)/
def taskStart = lines.get(10) =~ /measured build #1,\d+(?:\.\d+)?,(\d+(?:\.\d+)?)/
taskStart.matches()
Long.valueOf(taskStart[0][1]) > 0
Double.valueOf(taskStart[0][1]) > 0

where:
gradleVersion | configurationCache
Expand Down Expand Up @@ -98,12 +98,12 @@ class BuildOperationInstrumentationIntegrationTest extends AbstractProfilerInteg
lines.get(3) == "value,execution,SnapshotTaskInputsBuildOperationType"

def firstWarmup = lines.get(4)
def snapshottingDuration = firstWarmup =~ /warm-up build #1,\d+,(\d+)/
def snapshottingDuration = firstWarmup =~ /warm-up build #1,\d+(?:\.\d+)?,(\d+(?:\.\d+)?)/
snapshottingDuration.matches()
Long.valueOf(snapshottingDuration[0][1]) > 0
Double.valueOf(snapshottingDuration[0][1]) > 0

lines.get(9).matches("warm-up build #6,\\d+,\\d+")
lines.get(10).matches("measured build #1,\\d+,\\d+")
lines.get(9).matches(/warm-up build #6,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(10).matches(/measured build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)

where:
[via, commandLine, scenarioConfiguration, gradleVersion, configurationCache] << [
Expand Down Expand Up @@ -167,18 +167,18 @@ class BuildOperationInstrumentationIntegrationTest extends AbstractProfilerInteg
lines.get(1) == "version,Gradle ${gradleVersion},Gradle ${gradleVersion},Gradle ${gradleVersion}"
lines.get(2) == "tasks,assemble,assemble,assemble"
lines.get(3) == "value,execution,task start,SnapshotTaskInputsBuildOperationType"
lines.get(4).matches("warm-up build #1,\\d+,\\d+,\\d+")
lines.get(9).matches("warm-up build #6,\\d+,\\d+,\\d+")
lines.get(10).matches("measured build #1,\\d+,\\d+,\\d+")
lines.get(4).matches(/warm-up build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(9).matches(/warm-up build #6,\d+(?:\.\d+)?,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)
lines.get(10).matches(/measured build #1,\d+(?:\.\d+)?,\d+(?:\.\d+)?,\d+(?:\.\d+)?/)

and:
def buildLines = lines.subList(10, 19).collect { it.tokenize(',') }
def executions = buildLines.collect { Long.valueOf(it.get(1)) } as Set
def taskStarts = buildLines.collect { Long.valueOf(it.get(2)) } as Set
def buildOps = buildLines.collect { Long.valueOf(it.get(3)) } as Set
assertThat("non zero execution times", executions, hasItem(not(equalTo(0L))))
assertThat("non zero task start times", taskStarts, hasItem(not(equalTo(0L))))
assertThat("non zero build-op times", buildOps, hasItem(not(equalTo(0L))))
def executions = buildLines.collect { Double.valueOf(it.get(1)) } as Set
def taskStarts = buildLines.collect { Double.valueOf(it.get(2)) } as Set
def buildOps = buildLines.collect { Double.valueOf(it.get(3)) } as Set
assertThat("non zero execution times", executions, hasItem(not(equalTo(0d))))
assertThat("non zero task start times", taskStarts, hasItem(not(equalTo(0d))))
assertThat("non zero build-op times", buildOps, hasItem(not(equalTo(0d))))
assertTrue("different execution times", executions.size() > 1)
assertTrue("different task start times", taskStarts.size() > 1)
assertTrue("different build-op times", buildOps.size() > 1)
Expand Down