From 4d88fd74e7a72269aae54ae2b89534d055ade6f1 Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Mon, 28 Aug 2023 09:33:12 +0200 Subject: [PATCH 1/9] configurable headers + fix change diff sign --- scripts/ci/sizes.py | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/scripts/ci/sizes.py b/scripts/ci/sizes.py index a29967f0236f..f755dcec634b 100755 --- a/scripts/ci/sizes.py +++ b/scripts/ci/sizes.py @@ -95,7 +95,7 @@ def render(self, data: list[dict[str, str]]) -> str: return render_table_dict(data) -def compare(previous_path: str, current_path: str, threshold: float) -> None: +def compare(previous_path: str, current_path: str, threshold: float, before_header: str, after_header: str) -> None: previous = json.loads(Path(previous_path).read_text()) current = json.loads(Path(current_path).read_text()) @@ -109,21 +109,25 @@ def compare(previous_path: str, current_path: str, threshold: float) -> None: entries[name] = {} entries[name]["previous"] = entry - headers = ["Name", "Previous", "Current", "Change"] + headers = ["Name", before_header, after_header, "Change"] rows: list[tuple[str, str, str, str]] = [] for name, entry in entries.items(): if "previous" in entry and "current" in entry: previous = float(entry["previous"]["value"]) * DIVISORS[entry["previous"]["unit"]] current = float(entry["current"]["value"]) * DIVISORS[entry["current"]["unit"]] - min_change = previous * (threshold / 100) - unit = get_unit(min(previous, current)) div = get_divisor(unit) - change = ((previous / current) * 100) - 100 - sign = "+" if change > 0 else "" + change = abs(((current - previous) / previous) * 100) + if current < previous: + sign = "-" + elif current > previous: + sign = "+" + else: + sign = "" + min_change = previous * (threshold / 100) if abs(current - previous) >= min_change: rows.append( ( @@ -186,6 +190,20 @@ def main() -> None: default=20, help="Only print row if value is N%% larger or smaller", ) + compare_parser.add_argument( + "--before-header", + type=str, + required=False, + default="Before", + help="Header for before column", + ) + compare_parser.add_argument( + "--after-header", + type=str, + required=False, + default="After", + help="Header for after column", + ) measure_parser = cmds_parser.add_parser("measure", help="Measure sizes") measure_parser.add_argument( @@ -200,7 +218,13 @@ def main() -> None: args = parser.parse_args() if args.cmd == "compare": - compare(args.before, args.after, args.threshold) + compare( + args.before, + args.after, + args.threshold, + args.before_header, + args.after_header, + ) elif args.cmd == "measure": measure(args.files, args.format) From 52114cb8e03d11ccb325ff78bbc0579302395e87 Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Mon, 28 Aug 2023 09:33:26 +0200 Subject: [PATCH 2/9] use better headers when in ci --- .github/workflows/reusable_track_size.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/reusable_track_size.yml b/.github/workflows/reusable_track_size.yml index 8dd7f4d7ee80..bfd0c5f2332b 100644 --- a/.github/workflows/reusable_track_size.yml +++ b/.github/workflows/reusable_track_size.yml @@ -13,6 +13,7 @@ on: PR_NUMBER: required: false type: number + default: "" permissions: contents: write @@ -91,7 +92,13 @@ jobs: echo "$data" > "/tmp/data.json" - comparison=$(python3 scripts/ci/sizes.py compare --threshold=5 "/tmp/prev.json" "/tmp/data.json") + comparison=$( + python3 scripts/ci/sizes.py compare \ + --threshold=5 \ + --before-header=${{ (inputs.PR_NUMBER && github.event.pull_request.base.ref) || 'Before' }} \ + --after-header=${{ github.ref_name }} \ + "/tmp/prev.json" "/tmp/data.json" + ) echo "$comparison" { From 3a9619f511938cedd1daaaa925493ec37f6d3d0c Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:13:32 +0200 Subject: [PATCH 3/9] Use `+` when formatting to preserve sign --- scripts/ci/sizes.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/scripts/ci/sizes.py b/scripts/ci/sizes.py index f755dcec634b..bfda4144eb3e 100755 --- a/scripts/ci/sizes.py +++ b/scripts/ci/sizes.py @@ -119,13 +119,7 @@ def compare(previous_path: str, current_path: str, threshold: float, before_head unit = get_unit(min(previous, current)) div = get_divisor(unit) - change = abs(((current - previous) / previous) * 100) - if current < previous: - sign = "-" - elif current > previous: - sign = "+" - else: - sign = "" + change = ((current - previous) / previous) * 100 min_change = previous * (threshold / 100) if abs(current - previous) >= min_change: @@ -134,7 +128,7 @@ def compare(previous_path: str, current_path: str, threshold: float, before_head name, f"{cell(previous, div)} {unit}", f"{cell(current, div)} {unit}", - f"{sign}{change:.2f}%", + f"{change:+.2f}%", ) ) elif "current" in entry: From 66ec19385f684045f82499b57fc8986c2fb4c90b Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:16:03 +0200 Subject: [PATCH 4/9] update variable name --- scripts/ci/sizes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ci/sizes.py b/scripts/ci/sizes.py index bfda4144eb3e..189abb5754eb 100755 --- a/scripts/ci/sizes.py +++ b/scripts/ci/sizes.py @@ -119,7 +119,7 @@ def compare(previous_path: str, current_path: str, threshold: float, before_head unit = get_unit(min(previous, current)) div = get_divisor(unit) - change = ((current - previous) / previous) * 100 + change_pct = ((current - previous) / previous) * 100 min_change = previous * (threshold / 100) if abs(current - previous) >= min_change: @@ -128,7 +128,7 @@ def compare(previous_path: str, current_path: str, threshold: float, before_head name, f"{cell(previous, div)} {unit}", f"{cell(current, div)} {unit}", - f"{change:+.2f}%", + f"{change_pct:+.2f}%", ) ) elif "current" in entry: From f264a29357060c55bab1fcfd78e4b1f162da78bc Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:17:21 +0200 Subject: [PATCH 5/9] fix workflow --- .github/workflows/reusable_track_size.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/reusable_track_size.yml b/.github/workflows/reusable_track_size.yml index bfd0c5f2332b..068e3bf8fd13 100644 --- a/.github/workflows/reusable_track_size.yml +++ b/.github/workflows/reusable_track_size.yml @@ -13,7 +13,6 @@ on: PR_NUMBER: required: false type: number - default: "" permissions: contents: write From 69d8de1f960d15d2a0803e8d2e0e1d1d811311ff Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Tue, 29 Aug 2023 10:12:24 +0200 Subject: [PATCH 6/9] try to improve names --- scripts/ci/sizes.py | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/scripts/ci/sizes.py b/scripts/ci/sizes.py index 189abb5754eb..9b0ac356d377 100755 --- a/scripts/ci/sizes.py +++ b/scripts/ci/sizes.py @@ -53,10 +53,6 @@ def get_divisor(unit: str) -> int: return DIVISORS[unit] -def cell(value: float, div: float) -> str: - return f"{value / div:.2f}" - - def render_table_dict(data: list[dict[str, str]]) -> str: keys = data[0].keys() column_widths = [max(len(key), max(len(str(row[key])) for row in data)) for key in keys] @@ -95,15 +91,21 @@ def render(self, data: list[dict[str, str]]) -> str: return render_table_dict(data) -def compare(previous_path: str, current_path: str, threshold: float, before_header: str, after_header: str) -> None: - previous = json.loads(Path(previous_path).read_text()) - current = json.loads(Path(current_path).read_text()) +def compare( + previous_path: str, + current_path: str, + threshold_pct: float, + before_header: str, + after_header: str, +) -> None: + previous_bytes = json.loads(Path(previous_path).read_text()) + current_bytes = json.loads(Path(current_path).read_text()) entries = {} - for entry in current: + for entry in current_bytes: name = entry["name"] entries[name] = {"current": entry} - for entry in previous: + for entry in previous_bytes: name = entry["name"] if name not in entries: entries[name] = {} @@ -113,21 +115,22 @@ def compare(previous_path: str, current_path: str, threshold: float, before_head rows: list[tuple[str, str, str, str]] = [] for name, entry in entries.items(): if "previous" in entry and "current" in entry: - previous = float(entry["previous"]["value"]) * DIVISORS[entry["previous"]["unit"]] - current = float(entry["current"]["value"]) * DIVISORS[entry["current"]["unit"]] - - unit = get_unit(min(previous, current)) + previous_bytes = float(entry["previous"]["value"]) * DIVISORS[entry["previous"]["unit"]] + current_bytes = float(entry["current"]["value"]) * DIVISORS[entry["current"]["unit"]] + unit = get_unit(min(previous_bytes, current_bytes)) div = get_divisor(unit) - change_pct = ((current - previous) / previous) * 100 - - min_change = previous * (threshold / 100) - if abs(current - previous) >= min_change: + abs_diff_bytes = abs(current_bytes - previous_bytes) + min_diff_bytes = previous_bytes * (threshold_pct / 100) + if abs_diff_bytes >= min_diff_bytes: + previous = previous_bytes / div + current = current_bytes / div + change_pct = ((current_bytes - previous_bytes) / previous_bytes) * 100 rows.append( ( name, - f"{cell(previous, div)} {unit}", - f"{cell(current, div)} {unit}", + f"{previous:.2f} {unit}", + f"{current:.2f} {unit}", f"{change_pct:+.2f}%", ) ) From d0da78f5c3880dc695f4176f5f8fb45007cf3f98 Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Tue, 29 Aug 2023 10:15:00 +0200 Subject: [PATCH 7/9] more naming --- scripts/ci/sizes.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/ci/sizes.py b/scripts/ci/sizes.py index 9b0ac356d377..8cdb2918240c 100755 --- a/scripts/ci/sizes.py +++ b/scripts/ci/sizes.py @@ -98,14 +98,14 @@ def compare( before_header: str, after_header: str, ) -> None: - previous_bytes = json.loads(Path(previous_path).read_text()) - current_bytes = json.loads(Path(current_path).read_text()) + previous = json.loads(Path(previous_path).read_text()) + current = json.loads(Path(current_path).read_text()) entries = {} - for entry in current_bytes: + for entry in current: name = entry["name"] entries[name] = {"current": entry} - for entry in previous_bytes: + for entry in previous: name = entry["name"] if name not in entries: entries[name] = {} From cb697ebde189933bca66607855257ea6b56dcc0e Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Tue, 29 Aug 2023 11:08:42 +0200 Subject: [PATCH 8/9] track latest commit using index file --- .github/workflows/reusable_track_size.yml | 71 +++++++++++++---------- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/.github/workflows/reusable_track_size.yml b/.github/workflows/reusable_track_size.yml index 068e3bf8fd13..7ba30ddbdec3 100644 --- a/.github/workflows/reusable_track_size.yml +++ b/.github/workflows/reusable_track_size.yml @@ -6,10 +6,6 @@ on: CONCURRENCY: required: true type: string - ADHOC_NAME: - required: false - type: string - default: "" PR_NUMBER: required: false type: number @@ -30,14 +26,7 @@ jobs: - name: Get context id: context run: | - echo "head_short_sha=$(echo ${{ github.sha }} | cut -c1-7)" >> "$GITHUB_OUTPUT" - - if [ -n ${{ inputs.PR_NUMBER }} ]; then - base_short_sha=$(echo ${{ github.event.pull_request.base.sha }} | cut -c1-7) - else - base_short_sha=$short_sha - fi - echo "base_short_sha=$base_short_sha" >> "$GITHUB_OUTPUT" + echo "short_sha=$(echo ${{ github.sha }} | cut -c1-7)" >> "$GITHUB_OUTPUT" - id: "auth" uses: google-github-actions/auth@v1 @@ -62,12 +51,29 @@ jobs: name: web_demo path: web_demo - - name: Download base branch results + - name: Download base results run: | - file_path="gs://rerun-builds/sizes/commit/${{ steps.context.outputs.base_short_sha }}/data.json" + # Get base commit: + # 1. From the index file + # 2. From the latest commit in the base branch of the PR + # 3. From the latest commit in the current branch + index_path="gs://rerun-builds/sizes/index" + if [ "$(gsutil -q stat $index_path ; echo $?)" = 0 ]; then + gsutil cp $index_path "/tmp/base_index" + base_commit=$(cat /tmp/base_index) + else + if [ -n ${{ inputs.PR_NUMBER }} ]; then + base_commit=$(echo ${{ github.event.pull_request.base.sha }} | cut -c1-7) + else + base_commit=${{ steps.context.outputs.short_sha }} + fi + fi + echo "base commit: $base_commit" - if [ "$(gsutil -q stat $file_path ; echo $?)" = 0 ]; then - gsutil cp $file_path "/tmp/prev.json" + # Download data for base commit, or default to empty file + data_path="gs://rerun-builds/sizes/commit/$base_commit/data.json" + if [ "$(gsutil -q stat $data_path ; echo $?)" = 0 ]; then + gsutil cp $data_path "/tmp/prev.json" else echo "[]" > "/tmp/prev.json" fi @@ -87,8 +93,6 @@ jobs: done data=$(python3 scripts/ci/sizes.py measure "${entries[@]}") - echo "$data" - echo "$data" > "/tmp/data.json" comparison=$( @@ -98,8 +102,6 @@ jobs: --after-header=${{ github.ref_name }} \ "/tmp/prev.json" "/tmp/data.json" ) - echo "$comparison" - { echo 'comparison<> "$GITHUB_OUTPUT" fi - - name: Upload data to GCS (commit) - if: inputs.ADHOC_NAME == '' - uses: google-github-actions/upload-cloud-storage@v1 - with: - path: /tmp/data.json - destination: "rerun-builds/sizes/commit/${{ steps.context.outputs.head_short_sha }}" + echo "$entries" + echo "previous: $(cat /tmp/prev.json)" + echo "current: $(cat /tmp/data.json)" + echo "$comparison" + echo "is comparison set: $is_comparison_set" - - name: Upload data to GCS (adhoc) - if: inputs.ADHOC_NAME != '' + - name: Upload data to GCS (commit) uses: google-github-actions/upload-cloud-storage@v1 with: path: /tmp/data.json - destination: "rerun-builds/sizes/adhoc/${{ inputs.ADHOC_NAME }}" + destination: "rerun-builds/sizes/commit/${{ steps.context.outputs.short_sha }}" - name: Store result - # Only run on `main` if: github.ref == 'refs/heads/main' # https://github.com/benchmark-action/github-action-benchmark uses: benchmark-action/github-action-benchmark@v1 @@ -149,6 +148,18 @@ jobs: benchmark-data-dir-path: dev/sizes max-items-in-chart: 30 + - name: Create index file + if: github.ref == 'refs/heads/main' + run: | + echo "${{ steps.context.outputs.short_sha }}" > "/tmp/index" + + - name: Upload index + if: github.ref == 'refs/heads/main' + uses: google-github-actions/upload-cloud-storage@v1 + with: + path: /tmp/index + destination: "rerun-builds/sizes" + - name: Create PR comment if: inputs.PR_NUMBER != '' && steps.measure.outputs.is_comparison_set == 'true' # https://github.com/mshick/add-pr-comment From 941ec5ee46923b85efdcb1ecfd654c7ec8e679a0 Mon Sep 17 00:00:00 2001 From: jprochazk <1665677+jprochazk@users.noreply.github.com> Date: Tue, 29 Aug 2023 11:29:34 +0200 Subject: [PATCH 9/9] support `N%` values in `--threshold` --- .github/workflows/reusable_track_size.yml | 2 +- scripts/ci/sizes.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reusable_track_size.yml b/.github/workflows/reusable_track_size.yml index 7ba30ddbdec3..1dfae16de867 100644 --- a/.github/workflows/reusable_track_size.yml +++ b/.github/workflows/reusable_track_size.yml @@ -97,7 +97,7 @@ jobs: comparison=$( python3 scripts/ci/sizes.py compare \ - --threshold=5 \ + --threshold=5% \ --before-header=${{ (inputs.PR_NUMBER && github.event.pull_request.base.ref) || 'Before' }} \ --after-header=${{ github.ref_name }} \ "/tmp/prev.json" "/tmp/data.json" diff --git a/scripts/ci/sizes.py b/scripts/ci/sizes.py index 8cdb2918240c..a238acc0a1c3 100755 --- a/scripts/ci/sizes.py +++ b/scripts/ci/sizes.py @@ -172,6 +172,11 @@ def measure(files: list[str], format: Format) -> None: sys.stdout.flush() +def percentage(value: str) -> int: + value = value.replace("%", "") + return int(value) + + def main() -> None: parser = argparse.ArgumentParser(description="Generate a PR summary page") @@ -182,7 +187,7 @@ def main() -> None: compare_parser.add_argument("after", type=str, help="Current result .json file") compare_parser.add_argument( "--threshold", - type=float, + type=percentage, required=False, default=20, help="Only print row if value is N%% larger or smaller",