From dd3a21a976474e5ba0a341b7bb0f64612a328826 Mon Sep 17 00:00:00 2001 From: Son Luong Ngoc Date: Mon, 15 Jul 2024 14:11:49 +0200 Subject: [PATCH] invocation suggestion: add compression threshold (#7009) Co-authored-by: Brandon Duffany --- .bazelrc | 12 ++-- app/invocation/invocation_suggestion_card.tsx | 61 +++++++++++++++---- 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/.bazelrc b/.bazelrc index 718f485217e..c8d2424a4bc 100644 --- a/.bazelrc +++ b/.bazelrc @@ -18,14 +18,16 @@ common:cache-dev --bes_results_url=https://buildbuddy.buildbuddy.dev/invocation/ common:cache-dev --bes_backend=grpcs://buildbuddy.buildbuddy.dev common:cache-dev --remote_cache=grpcs://buildbuddy.buildbuddy.dev common:cache-dev --remote_upload_local_results -common:cache-dev --experimental_remote_cache_compression +common:cache-dev --remote_cache_compression +common:cache-dev --experimental_remote_cache_compression_threshold=100 # Build with --config=cache to send build logs to the production server with cache common:cache --bes_results_url=https://buildbuddy.buildbuddy.io/invocation/ common:cache --bes_backend=grpcs://buildbuddy.buildbuddy.io common:cache --remote_cache=grpcs://buildbuddy.buildbuddy.io common:cache --remote_upload_local_results -common:cache --experimental_remote_cache_compression +common:cache --remote_cache_compression +common:cache --experimental_remote_cache_compression_threshold=100 # Flags shared across remote configs common:remote-shared --remote_upload_local_results @@ -80,11 +82,13 @@ common:untrusted-ci-windows --flaky_test_attempts=2 common:untrusted-ci-windows --bes_results_url=https://app.buildbuddy.io/invocation/ common:untrusted-ci-windows --bes_backend=grpcs://remote.buildbuddy.io common:untrusted-ci-windows --remote_cache=grpcs://remote.buildbuddy.io -common:untrusted-ci-windows --experimental_remote_cache_compression +common:untrusted-ci-windows --remote_cache_compression +common:untrusted-ci-windows --experimental_remote_cache_compression_threshold=100 common:untrusted-ci-windows --remote_upload_local_results # Configuration used for all BuildBuddy workflows -common:workflows --experimental_remote_cache_compression +common:workflows --remote_cache_compression +common:workflows --experimental_remote_cache_compression_threshold=100 common:workflows --build_metadata=ROLE=CI common:workflows --build_metadata=VISIBILITY=PUBLIC common:workflows --remote_instance_name=buildbuddy-io/buildbuddy/workflows diff --git a/app/invocation/invocation_suggestion_card.tsx b/app/invocation/invocation_suggestion_card.tsx index 6aa44c8e3e5..d7d7645ea68 100644 --- a/app/invocation/invocation_suggestion_card.tsx +++ b/app/invocation/invocation_suggestion_card.tsx @@ -327,23 +327,56 @@ ${yamlSuggestions.map((s) => ` ${s}`).join("\n")}`} if (model.optionsMap.get("remote_cache_compression")) return null; if (model.optionsMap.get("experimental_remote_cache_compression")) return null; if (!model.optionsMap.get("remote_cache") && !model.optionsMap.get("remote_executor")) return null; - const version = getBazelMajorVersion(model); + + const version = getBazelVersion(model); // Bazel pre-v5 doesn't support compression. - if (version === null || version < 5) return null; + if (version === null || version.major < 5) return null; + + const flag = version.major >= 7 ? "--experimental_remote_cache_compression" : "--remote_cache_compression"; return { level: SuggestionLevel.INFO, message: ( <> - Consider adding the Bazel flag --experimental_remote_cache_compression to improve - remote cache throughput. + Consider adding the Bazel flag {flag} to improve remote cache throughput. ), reason: ( <> - Shown because this build is cache-enabled but{" "} - --experimental_remote_cache_compression is neither enabled nor explicitly - disabled. + Shown because this build is cache-enabled but {flag} is neither enabled + nor explicitly disabled. + + ), + }; + }, + ({ model }) => { + if (!capabilities.config.expandedSuggestionsEnabled) return null; + if (!model.isBazelInvocation()) return null; + + if (model.optionsMap.get("experimental_remote_cache_compression_threshold")) return null; + if ( + !model.optionsMap.get("remote_cache_compression") && + !model.optionsMap.get("experimental_remote_cache_compression") + ) + return null; + if (!model.optionsMap.get("remote_cache") && !model.optionsMap.get("remote_executor")) return null; + + const version = getBazelVersion(model); + // threshold flag is available from Bazel 7.1 forward + if (version === null || version.major < 7 || version.minor < 1) return null; + + return { + level: SuggestionLevel.INFO, + message: ( + <> + Consider adding the Bazel flag --experimental_remote_cache_compression_threshold=100 to + avoid inflating blobs smaller than 100 bytes with ZSTD compression. + + ), + reason: ( + <> + Shown because this build is cache-enabled with --remote_cache_compression{" "} + set without --experimental_remote_cache_compression_threshold set. ), }; @@ -376,10 +409,10 @@ ${yamlSuggestions.map((s) => ` ${s}`).join("\n")}`} if (!model.optionsMap.get("remote_cache")) return null; if (model.optionsMap.get("remote_build_event_upload")) return null; if (model.optionsMap.get("experimental_remote_build_event_upload")) return null; - const version = getBazelMajorVersion(model); + const version = getBazelVersion(model); // Bazel pre-v6 doesn't support --experimental_remote_build_event_upload=minimal, and Bazel post-v6 default to the // correct setting - if (version === null || version != 6) return null; + if (version === null || version.major != 6) return null; return { level: SuggestionLevel.INFO, @@ -689,10 +722,14 @@ function InlineProseList({ items }: { items: React.ReactNode[] }) { return <>{out}; } -function getBazelMajorVersion(model: InvocationModel): number | null { +// getBazelVersion returns the major and minor version of Bazel from BES event. +// +// The version could contain rc version in the patch number, such as "7.2.1rc1". +function getBazelVersion(model: InvocationModel): { major: number; minor: number } | null { const version = model.started?.buildToolVersion; if (!version) return null; const segments = version.split(".").map(Number); - if (isNaN(segments[0])) return null; - return segments[0]; + if (segments.length < 2) return null; + if (segments.slice(0, 2).some(isNaN)) return null; + return { major: segments[0], minor: segments[1] }; }