diff --git a/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/FetchBuildValidationDataCommand.java b/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/FetchBuildValidationDataCommand.java index 2ca3e4df..29b3fb19 100644 --- a/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/FetchBuildValidationDataCommand.java +++ b/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/FetchBuildValidationDataCommand.java @@ -11,11 +11,9 @@ import picocli.CommandLine.Parameters; import java.io.IOException; -import java.math.BigDecimal; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Path; -import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -108,8 +106,8 @@ private BuildValidationData fetchBuildScanData(int index, URL buildScanUrl, Cust "", null, Collections.emptyMap(), - Duration.ZERO, - BigDecimal.ZERO); + null, + null); } } diff --git a/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/Fields.java b/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/Fields.java index b9a12ef7..c7ed2f1a 100644 --- a/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/Fields.java +++ b/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/cli/Fields.java @@ -6,7 +6,6 @@ import java.math.BigDecimal; import java.time.Duration; import java.util.Arrays; -import java.util.Locale; import java.util.function.Function; import java.util.stream.Stream; @@ -31,11 +30,12 @@ public enum Fields { EXECUTED_CACHEABLE_DURATION("Executed cacheable duration", d -> totalDuration(d, "executed_cacheable")), EXECUTED_NOT_CACHEABLE("Executed not cacheable", d -> totalTasks(d, "executed_not_cacheable")), EXECUTED_NOT_CACHEABLE_DURATION("Executed not cacheable duration", d -> totalDuration(d, "executed_not_cacheable")), - EFFECTIVE_TASK_EXECUTION_DURATION("Effective task execution duration", d -> String.valueOf(d.getEffectiveTaskExecutionDuration().toMillis())), - SERIALIZATION_FACTOR("Serialization factor", d -> formatBigDecimal(d.getSerializationFactor())), - EXECUTED_CACHEABLE_DURATION_MILLISECONDS("Executed cacheable duration milliseconds", d -> totalDurationMillis(d, "executed_cacheable")), + EFFECTIVE_TASK_EXECUTION_DURATION("Effective task execution duration", d -> formatDuration(d.getEffectiveTaskExecutionDuration())), + SERIALIZATION_FACTOR("Serialization factor", d -> toStringSafely(d.getSerializationFactor())), ; + private static final String NO_VALUE = ""; + public final String label; public final Function value; @@ -49,65 +49,33 @@ public static Stream ordered() { } private static String toStringSafely(Object object) { - if (object == null) { - return ""; - } - return object.toString(); + return object == null ? NO_VALUE : object.toString(); + } + + private static String toStringSafely(BigDecimal value) { + return value == null ? NO_VALUE : value.toPlainString(); } private static String totalTasks(BuildValidationData data, String avoidanceOutcome) { - if (data.getTasksByAvoidanceOutcome().containsKey(avoidanceOutcome)) { - return data.getTasksByAvoidanceOutcome().get(avoidanceOutcome).totalTasks().toString(); - } - return ""; + return summaryTotal(data, avoidanceOutcome, t -> String.valueOf(t.totalTasks())); } private static String totalAvoidanceSavings(BuildValidationData data, String avoidanceOutcome) { - return formatDuration( - data.getTasksByAvoidanceOutcome() - .getOrDefault(avoidanceOutcome, TaskExecutionSummary.ZERO) - .totalAvoidanceSavings() - ); + return summaryTotal(data, avoidanceOutcome, t -> formatDuration(t.totalAvoidanceSavings())); } private static String totalDuration(BuildValidationData data, String avoidanceOutcome) { - return formatDuration( - data.getTasksByAvoidanceOutcome() - .getOrDefault(avoidanceOutcome, TaskExecutionSummary.ZERO) - .totalDuration() - ); - } - - private static String totalDurationMillis(BuildValidationData data, String avoidanceOutcome) { - return String.valueOf( - data.getTasksByAvoidanceOutcome() - .getOrDefault(avoidanceOutcome, TaskExecutionSummary.ZERO) - .totalDuration() - .toMillis() - ); + return summaryTotal(data, avoidanceOutcome, t -> formatDuration(t.totalDuration())); } - private static String formatDuration(Duration duration) { - long hours = duration.toHours(); - long minutes = duration.minusHours(hours).toMinutes(); - double seconds = duration.minusHours(hours).minusMinutes(minutes).toMillis() / 1000d; - - StringBuilder s = new StringBuilder(); - if (hours != 0) { - s.append(hours + "h "); - } - if (minutes != 0) { - s.append(minutes + "m "); + private static String summaryTotal(BuildValidationData data, String avoidanceOutcome, Function toString) { + if (data.getTasksByAvoidanceOutcome().containsKey(avoidanceOutcome)) { + return toString.apply(data.getTasksByAvoidanceOutcome().get(avoidanceOutcome)); } - s.append(String.format(Locale.ROOT, "%.3fs", seconds)); - - return s.toString().trim(); + return NO_VALUE; } - private static String formatBigDecimal(BigDecimal value) { - if (value.compareTo(BigDecimal.ZERO) == 0) { - return ""; - } - return value.toPlainString(); + private static String formatDuration(Duration duration) { + return duration == null ? NO_VALUE : String.valueOf(duration.toMillis()); } } diff --git a/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/model/BuildValidationData.java b/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/model/BuildValidationData.java index 7c5e3cf3..ce332451 100644 --- a/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/model/BuildValidationData.java +++ b/components/fetch-build-scan-data-cmdline-tool/src/main/java/com/gradle/enterprise/model/BuildValidationData.java @@ -56,10 +56,6 @@ public String getRootProjectName() { return rootProjectName; } - public boolean isRootProjectNameFound() { - return isFound(rootProjectName); - } - public String getBuildScanId() { return buildScanId; } @@ -104,26 +100,14 @@ public List getRequestedTasks() { return requestedTasks; } - public boolean isRequestedTasksFound() { - return !requestedTasks.isEmpty(); - } - public String getBuildOutcome() { return buildOutcome; } - public boolean isBuildOutcomeFound() { - return isFound(buildOutcome); - } - public URL getRemoteBuildCacheUrl() { return remoteBuildCacheUrl; } - public boolean isRemoteBuildCacheUrlFound() { - return remoteBuildCacheUrl != null; - } - public String getRemoteBuildCacheShard() { if (remoteBuildCacheUrl == null) { return ""; diff --git a/components/scripts/gradle/01-validate-incremental-building.sh b/components/scripts/gradle/01-validate-incremental-building.sh index 14dd436a..75f50f05 100755 --- a/components/scripts/gradle/01-validate-incremental-building.sh +++ b/components/scripts/gradle/01-validate-incremental-building.sh @@ -161,11 +161,6 @@ fetch_build_cache_metrics() { fi } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - # Overrides info.sh#print_performance_characteristics print_performance_characteristics() { print_performance_characteristics_header diff --git a/components/scripts/gradle/02-validate-local-build-caching-same-location.sh b/components/scripts/gradle/02-validate-local-build-caching-same-location.sh index 8a563301..377b00d9 100755 --- a/components/scripts/gradle/02-validate-local-build-caching-same-location.sh +++ b/components/scripts/gradle/02-validate-local-build-caching-same-location.sh @@ -164,11 +164,6 @@ fetch_build_cache_metrics() { fi } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/gradle/03-validate-local-build-caching-different-locations.sh b/components/scripts/gradle/03-validate-local-build-caching-different-locations.sh index c82f75bf..690e452b 100755 --- a/components/scripts/gradle/03-validate-local-build-caching-different-locations.sh +++ b/components/scripts/gradle/03-validate-local-build-caching-different-locations.sh @@ -169,11 +169,6 @@ fetch_build_cache_metrics() { fi } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/gradle/04-validate-remote-build-caching-ci-ci.sh b/components/scripts/gradle/04-validate-remote-build-caching-ci-ci.sh index 1b2f5677..56be8661 100755 --- a/components/scripts/gradle/04-validate-remote-build-caching-ci-ci.sh +++ b/components/scripts/gradle/04-validate-remote-build-caching-ci-ci.sh @@ -153,11 +153,6 @@ print_experiment_specific_summary_info() { summary_row "Custom value mapping file:" "${mapping_file:-}" } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh b/components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh index dddeb65c..f4f1cd64 100755 --- a/components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh +++ b/components/scripts/gradle/05-validate-remote-build-caching-ci-local.sh @@ -213,11 +213,6 @@ print_experiment_specific_summary_info() { summary_row "Custom value mapping file:" "${mapping_file:-}" } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/lib/build-scan-parse.sh b/components/scripts/lib/build-scan-parse.sh index 9dd87671..5e1364cc 100644 --- a/components/scripts/lib/build-scan-parse.sh +++ b/components/scripts/lib/build-scan-parse.sh @@ -28,7 +28,6 @@ executed_not_cacheable_duration=() # Build duration metrics effective_task_execution_duration=() serialization_factors=() -executed_cacheable_duration_milliseconds=() parse_build_scan_csv() { # This isn't the most robust way to read a CSV, @@ -47,7 +46,7 @@ parse_build_scan_csv() { idx=0 # shellcheck disable=SC2034 # not all scripts use all of the fetched data - while IFS=, read -r field_1 field_2 field_3 field_4 field_5 field_6 field_7 field_8 field_9 field_10 field_11 field_12 field_13 field_14 field_15 field_16 field_17 field_18 field_19 field_20 field_21 field_22; do + while IFS=, read -r field_1 field_2 field_3 field_4 field_5 field_6 field_7 field_8 field_9 field_10 field_11 field_12 field_13 field_14 field_15 field_16 field_17 field_18 field_19 field_20 field_21; do if [[ "$header_row_read" == "false" ]]; then header_row_read=true continue; @@ -78,18 +77,9 @@ parse_build_scan_csv() { executed_not_cacheable_num_tasks[idx]="${field_18}" executed_not_cacheable_duration[idx]="${field_19}" - # Conditional because build-scan-support-tool does not yet support these fields - if [[ -n "$field_20" ]]; then - effective_task_execution_duration[idx]="${field_20}" - fi - - if [[ -n "$field_21" ]]; then - serialization_factors[idx]="${field_21}" - fi - - if [[ -n "$field_22" ]]; then - executed_cacheable_duration_milliseconds[idx]="${field_22}" - fi + # Build duration metrics + effective_task_execution_duration[idx]="${field_20}" + serialization_factors[idx]="${field_21}" ((idx++)) done <<< "${build_scan_csv}" diff --git a/components/scripts/lib/info.sh b/components/scripts/lib/info.sh index ee56e803..c44206c0 100644 --- a/components/scripts/lib/info.sh +++ b/components/scripts/lib/info.sh @@ -97,8 +97,10 @@ print_summary() { print_experiment_info print_experiment_specific_summary_info print_build_scans + print_warnings - print_performance_metrics + + print_performance_characteristics if [[ "${build_scan_publishing_mode}" == "on" ]]; then print_bl @@ -129,12 +131,10 @@ print_experiment_specific_summary_info() { true } -print_performance_metrics() { - # this function is intended to be overridden by experiments as-needed - # have one command to satisfy shellcheck - true -} - +# This function is responsible for printing the "Performance Characteristics" +# section of the experiment summary. +# +# Experiments may override this function to include only relevant metrics. print_performance_characteristics() { print_performance_characteristics_header @@ -155,50 +155,43 @@ print_performance_characteristics_header() { info "---------------------------" } +# The _realized_ build time savings is the difference in the wall-clock build +# time between the first and second build. print_realized_build_time_savings() { - # Do not print realized build time savings at all if these values do not exist - # This can happen since build-scan-support-tool does not yet support these fields + local value + # Only calculate realized build time savings when these values exist + # These values can be returned as empty when an error occurs processing the Build Scan data if [[ -n "${effective_task_execution_duration[0]}" && -n "${effective_task_execution_duration[1]}" ]]; then - local value - value="" - # Only calculate realized build time savings when these values are non-zero - # These values can be returned as zero when an error occurs processing the Build Scan data - if [[ "${effective_task_execution_duration[0]}" && "${effective_task_execution_duration[1]}" ]]; then - local first_build second_build realized_savings - first_build=$(format_duration "${effective_task_execution_duration[0]}") - second_build=$(format_duration "${effective_task_execution_duration[1]}") - realized_savings=$(format_duration effective_task_execution_duration[0]-effective_task_execution_duration[1]) - value="${realized_savings} wall-clock time (from ${first_build} to ${second_build})" - fi - summary_row "Realized build time savings:" "${value}" + local realized_build_time_savings=$((effective_task_execution_duration[0]-effective_task_execution_duration[1])) + printf -v value "%s wall-clock time (from %s to %s)" \ + "$(format_duration "${realized_build_time_savings}")" \ + "$(format_duration "${effective_task_execution_duration[0]}")" \ + "$(format_duration "${effective_task_execution_duration[1]}")" fi + summary_row "Realized build time savings:" "${value}" } +# The _potential_ build time savings is the difference in wall-clock build time +# between the first build and the _potential_ build time of the second build. +# +# The _potential_ build time is an estimation of the build time if no cacheable +# tasks had been executed. print_potential_build_time_savings() { - local build_1_effective_execution_duration="${effective_task_execution_duration[0]}" - - local build_2_effective_execution_duration="${effective_task_execution_duration[1]}" - local build_2_executed_cacheable_duration="${executed_cacheable_duration_milliseconds[1]}" - local build_2_serialization_factor="${serialization_factors[1]}" - - # Do not print potential build time savings at all if these values do not exist - # This can happen since build-scan-support-tool does not yet support these fields - if [[ -z "${build_1_effective_execution_duration}" || -z "${build_2_effective_execution_duration}" || -z "${build_2_executed_cacheable_duration}" || -z "${build_2_serialization_factor}" ]]; then - return 0 - fi - local value - value="" - # Only calculate realized build time savings when these have valid values - # These values can be returned as zero when an error occurs processing the Build Scan data - if [[ "${build_1_effective_execution_duration}" && "${build_2_effective_execution_duration}" && -n "${build_2_serialization_factor}" ]]; then - local first_build second_build potential_build_duration potential_savings - first_build=$(format_duration "${effective_task_execution_duration[0]}") - # shellcheck disable=SC2034 # it's used on the next few lines - potential_build_duration=$(echo "${build_2_effective_execution_duration}-(${build_2_executed_cacheable_duration}/${build_2_serialization_factor})" | bc) - potential_savings=$(format_duration build_1_effective_execution_duration-potential_build_duration) - second_build=$(format_duration potential_build_duration) - value="${potential_savings} wall-clock time (from ${first_build} to ${second_build})" + # Only calculate realized build time savings when these values exist + # These values can be returned as empty when an error occurs processing the Build Scan data + if [[ -n "${effective_task_execution_duration[0]}" && \ + -n "${effective_task_execution_duration[1]}" && \ + -n "${executed_cacheable_duration[1]}" && \ + -n "${serialization_factors[1]}" ]] + then + local potential_build_time potential_build_time_savings + potential_build_time=$(echo "${effective_task_execution_duration[1]}-(${executed_cacheable_duration[1]}/${serialization_factors[1]})" | bc) + potential_build_time_savings=$((effective_task_execution_duration[0]-potential_build_time)) + printf -v value "%s wall-clock time (from %s to %s)" \ + "$(format_duration "${potential_build_time_savings}")" \ + "$(format_duration "${effective_task_execution_duration[0]}")" \ + "$(format_duration "${potential_build_time}")" fi summary_row "Potential build time savings:" "${value}" } @@ -207,42 +200,57 @@ print_build_caching_leverage_metrics() { local task_count_padding task_count_padding=$(max_length "${avoided_from_cache_num_tasks[1]}" "${executed_cacheable_num_tasks[1]}" "${executed_not_cacheable_num_tasks[1]}") + print_avoided_cacheable_tasks "${task_count_padding}" + + print_executed_cacheable_tasks "${task_count_padding}" + + print_executed_non_cacheable_tasks "${task_count_padding}" +} + +print_avoided_cacheable_tasks() { local value - value="" - if [[ "${avoided_from_cache_num_tasks[1]}" ]]; then - local taskCount - taskCount="$(printf "%${task_count_padding}s" "${avoided_from_cache_num_tasks[1]}" )" - value="${taskCount} ${BUILD_TOOL_TASK}s, ${avoided_from_cache_avoidance_savings[1]} total saved execution time" + if [[ -n "${avoided_from_cache_num_tasks[1]}" && -n "${avoided_from_cache_avoidance_savings[1]}" ]]; then + printf -v value "%$1s %ss, %s total saved execution time" \ + "${avoided_from_cache_num_tasks[1]}" \ + "${BUILD_TOOL_TASK}" \ + "$(format_duration avoided_from_cache_avoidance_savings[1])" fi summary_row "Avoided cacheable ${BUILD_TOOL_TASK}s:" "${value}" +} - value="" - if [[ "${executed_cacheable_num_tasks[1]}" ]]; then +print_executed_cacheable_tasks() { + local value + if [[ -n "${executed_cacheable_num_tasks[1]}" && -n "${executed_cacheable_duration[1]}" ]]; then local summary_color - summary_color="" if (( executed_cacheable_num_tasks[1] > 0)); then summary_color="${WARN_COLOR}" fi - taskCount="$(printf "%${task_count_padding}s" "${executed_cacheable_num_tasks[1]}" )" - value="${summary_color}${taskCount} ${BUILD_TOOL_TASK}s, ${executed_cacheable_duration[1]} total execution time${RESTORE}" + printf -v value "${summary_color}%$1s %ss, %s total execution time${RESTORE}" \ + "${executed_cacheable_num_tasks[1]}" \ + "${BUILD_TOOL_TASK}" \ + "$(format_duration executed_cacheable_duration[1])" fi summary_row "Executed cacheable ${BUILD_TOOL_TASK}s:" "${value}" +} - value="" - if [[ "${executed_not_cacheable_num_tasks[1]}" ]]; then - taskCount="$(printf "%${task_count_padding}s" "${executed_not_cacheable_num_tasks[1]}" )" - value="${taskCount} ${BUILD_TOOL_TASK}s, ${executed_not_cacheable_duration[1]} total execution time" +print_executed_non_cacheable_tasks() { + local value + if [[ -n "${executed_not_cacheable_num_tasks[1]}" && -n "${executed_not_cacheable_duration[1]}" ]]; then + printf -v value "%$1s %ss, %s total execution time" \ + "${executed_not_cacheable_num_tasks[1]}" \ + "${BUILD_TOOL_TASK}" \ + "$(format_duration executed_not_cacheable_duration[1])" fi summary_row "Executed non-cacheable ${BUILD_TOOL_TASK}s:" "${value}" } print_serialization_factor() { - # Do not print serialization factor at all if this value does not exist - # This can happen since build-scan-support-tool does not yet support this field + local value if [[ -n "${serialization_factors[0]}" ]]; then - summary_row "Serialization factor:" "$(to_two_decimal_places "${serialization_factors[0]}")x" + value="$(to_two_decimal_places "${serialization_factors[0]}")x" fi + summary_row "Serialization factor:" "${value}" } print_executed_cacheable_tasks_warning() { diff --git a/components/scripts/maven/01-validate-local-build-caching-same-location.sh b/components/scripts/maven/01-validate-local-build-caching-same-location.sh index 80648673..5fe01484 100755 --- a/components/scripts/maven/01-validate-local-build-caching-same-location.sh +++ b/components/scripts/maven/01-validate-local-build-caching-same-location.sh @@ -165,11 +165,6 @@ fetch_build_cache_metrics() { fi } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/maven/02-validate-local-build-caching-different-locations.sh b/components/scripts/maven/02-validate-local-build-caching-different-locations.sh index 7cdea1c1..4828e936 100755 --- a/components/scripts/maven/02-validate-local-build-caching-different-locations.sh +++ b/components/scripts/maven/02-validate-local-build-caching-different-locations.sh @@ -169,11 +169,6 @@ fetch_build_cache_metrics() { fi } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/maven/03-validate-remote-build-caching-ci-ci.sh b/components/scripts/maven/03-validate-remote-build-caching-ci-ci.sh index 9b65453a..ee1cec06 100755 --- a/components/scripts/maven/03-validate-remote-build-caching-ci-ci.sh +++ b/components/scripts/maven/03-validate-remote-build-caching-ci-ci.sh @@ -151,11 +151,6 @@ print_experiment_specific_summary_info() { summary_row "Custom value mapping file:" "${mapping_file:-}" } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------" diff --git a/components/scripts/maven/04-validate-remote-build-caching-ci-local.sh b/components/scripts/maven/04-validate-remote-build-caching-ci-local.sh index b64009a4..8e62f522 100755 --- a/components/scripts/maven/04-validate-remote-build-caching-ci-local.sh +++ b/components/scripts/maven/04-validate-remote-build-caching-ci-local.sh @@ -212,11 +212,6 @@ print_experiment_specific_summary_info() { summary_row "Custom value mapping file:" "${mapping_file:-}" } -# Overrides info.sh#print_performance_metrics -print_performance_metrics() { - print_performance_characteristics -} - print_quick_links() { info "Investigation Quick Links" info "-------------------------"