From e2cffa69346ebf8f38ec0ee8e8d3f833ce59de94 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 14 May 2024 13:16:14 -0300 Subject: [PATCH 1/9] chore: Do not rebuild yarn-projects on bench-comment --- .github/workflows/ci.yml | 2 +- yarn-project/scripts/Earthfile | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 184848f54f4..6ae0d3c3f86 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -174,7 +174,7 @@ jobs: env: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - BENCH_FOLDER: ./yarn-project/scripts/bench + BENCH_FOLDER: /usr/var/bench PULL_REQUEST: "${{ github.event.pull_request.number }}" - name: "Generate summary comment if pull request" if: ${{ github.event_name == 'pull_request' }} diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index 98f856d6fac..d822bc4aab1 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -53,13 +53,14 @@ bench-comment: ARG PULL_REQUEST ARG BRANCH FROM +build + LET LOCAL_BASE_BENCH_FOLDER=/usr/var/bench LET BENCH_FOLDER=/usr/var/bench ENV BENCH_FOLDER=$BENCH_FOLDER ENV COMMIT_HASH=$COMMIT_HASH ENV PULL_REQUEST=$PULL_REQUEST ENV PR_NUMBER=$PULL_REQUEST ENV BRANCH=$BRANCH - COPY ./bench $BENCH_FOLDER/ + COPY $LOCAL_BASE_BENCH_FOLDER $BENCH_FOLDER/ COPY +bench-aggregate/bench $BENCH_FOLDER RUN echo "Bench folder contents $(ls $BENCH_FOLDER)" RUN --secret AZTEC_BOT_COMMENTER_GITHUB_TOKEN \ From bf25da4b4e4a8bd0a9b88ffc293028eb87335ea3 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 15 May 2024 09:28:10 -0300 Subject: [PATCH 2/9] Download base bench to home --- .github/workflows/ci.yml | 10 ++++++++-- yarn-project/scripts/Earthfile | 4 +--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ae0d3c3f86..accd28f17c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -174,12 +174,18 @@ jobs: env: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - BENCH_FOLDER: /usr/var/bench + BENCH_FOLDER: "${{ env.HOME }}/base_bench" PULL_REQUEST: "${{ github.event.pull_request.number }}" - name: "Generate summary comment if pull request" if: ${{ github.event_name == 'pull_request' }} working-directory: ./yarn-project/scripts - run: earthly-ci -P --secret AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }} --secret AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }} --secret AZTEC_BOT_COMMENTER_GITHUB_TOKEN=${{ secrets.AZTEC_BOT_GITHUB_TOKEN }} +bench-comment + run: | + earthly-ci -P \ + --secret AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }} \ + --secret AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }} \ + --secret AZTEC_BOT_COMMENTER_GITHUB_TOKEN=${{ secrets.AZTEC_BOT_GITHUB_TOKEN }} \ + +bench-comment \ + --LOCAL_BASE_BENCH_FOLDER="${{ env.HOME }}/base_bench" noir-format: needs: setup diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index d822bc4aab1..8e559f1804c 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -13,9 +13,7 @@ download-logs: ARG BRANCH FROM +build LET LOG_FOLDER=/usr/var/logs - LET BENCH_FOLDER=/usr/var/bench ENV LOG_FOLDER=$LOG_FOLDER - ENV BENCH_FOLDER=$BENCH_FOLDER ENV COMMIT_HASH=$COMMIT_HASH ENV PULL_REQUEST=$PULL_REQUEST ENV BRANCH=$BRANCH @@ -52,8 +50,8 @@ bench-comment: ARG COMMIT_HASH ARG PULL_REQUEST ARG BRANCH + ARG LOCAL_BASE_BENCH_FOLDER FROM +build - LET LOCAL_BASE_BENCH_FOLDER=/usr/var/bench LET BENCH_FOLDER=/usr/var/bench ENV BENCH_FOLDER=$BENCH_FOLDER ENV COMMIT_HASH=$COMMIT_HASH From ffa46658a1c345798aff03e0ea03aafe050f9e15 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 15 May 2024 14:40:54 -0300 Subject: [PATCH 3/9] Try another approach --- .github/workflows/ci.yml | 22 ++++++++++++++++------ yarn-project/scripts/Earthfile | 32 ++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index accd28f17c9..f232e2e61f0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -167,14 +167,25 @@ jobs: concurrency_key: build-x86 - name: "Build and upload bench aggregate file" working-directory: ./yarn-project/scripts - run: earthly-ci -P --secret AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }} --secret AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }} +bench-aggregate - - name: "Download base benchmark" + run: | + earthly-ci -P \ + --secret AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }} \ + --secret AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }} \ + +bench-aggregate + - name: "Download base benchmark and package into earthly" if: ${{ github.event_name == 'pull_request' }} - run: scripts/logs/download_base_benchmark_from_s3.sh + run: | + # Download the base benchmark locally (requires AWS creds and .git history) + scripts/logs/download_base_benchmark_from_s3.sh + cd yarn-project/scripts + # Package it into an earthly artifact + earthly-ci -P +pack-base-benchmark --LOCAL_BENCH_FOLDER="./bench" + # Delete it so we don't accidentally bust the yarn-project+build cache + rm -rf ./bench env: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - BENCH_FOLDER: "${{ env.HOME }}/base_bench" + BENCH_FOLDER: "./yarn-project/scripts/bench" PULL_REQUEST: "${{ github.event.pull_request.number }}" - name: "Generate summary comment if pull request" if: ${{ github.event_name == 'pull_request' }} @@ -184,8 +195,7 @@ jobs: --secret AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }} \ --secret AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }} \ --secret AZTEC_BOT_COMMENTER_GITHUB_TOKEN=${{ secrets.AZTEC_BOT_GITHUB_TOKEN }} \ - +bench-comment \ - --LOCAL_BASE_BENCH_FOLDER="${{ env.HOME }}/base_bench" + +bench-comment noir-format: needs: setup diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index 8e559f1804c..164c4d47ea8 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -41,16 +41,32 @@ bench-aggregate: || (echo "Not all log files from benchmark jobs found"; mkdir -p $BENCH_FOLDER) SAVE ARTIFACT $BENCH_FOLDER bench +pack-base-benchmark: + # Copies the base benchmark (ie the master run) into a container and packs it as an artifact, + # so it can be consumed from bench-comment. Note that we need to download base-benchmark + # outside of this target beforehand. We cannot run it within an Earthly container because it needs + # access to the .git folder, and we cannot run it with a LOCALLY statement because Earthly does + # not support secrets when running locally (and we need) the AWS access keys to access S3. + # We also cannot COPY the local file directly from bench-comment, since the file must be present + # within the build context so we can copy it (ie within yarn-project/scripts), and that invalidates + # the cache of yarn-project+build since it does a `COPY . .`, and we cannot add the bench file to + # earthlyignore or we would not be able to copy it from anywhere. + ARG COMMIT_HASH + ARG PULL_REQUEST + ARG BRANCH + ARG LOCAL_BENCH_FOLDER + FROM +scratch + LET BENCH_FOLDER=/usr/var/bench + COPY --if-exists $LOCAL_BENCH_FOLDER $BENCH_FOLDER + RUN mkdir -p $BENCH_FOLDER + SAVE ARTIFACT $BENCH_FOLDER bench + bench-comment: - # Use the scripts image to run bench comment after loading the benchmark from bench-aggregate. - # Requires base-benchmark to be downloaded outside of this target. Note we cannot run it within - # an Earthly container because it needs access to the .git folder, and we cannot run it with a - # LOCALLY statement because Earthly does not support secrets when running locally (and we need) - # the AWS access keys to access S3. This step is then manually run in the ci.yml. + # Use the scripts image to run bench comment after loading the benchmark from bench-aggregate + # and the base benchmark (ie the benches from master to compare to) from pack-base-benchmark. ARG COMMIT_HASH ARG PULL_REQUEST ARG BRANCH - ARG LOCAL_BASE_BENCH_FOLDER FROM +build LET BENCH_FOLDER=/usr/var/bench ENV BENCH_FOLDER=$BENCH_FOLDER @@ -58,8 +74,8 @@ bench-comment: ENV PULL_REQUEST=$PULL_REQUEST ENV PR_NUMBER=$PULL_REQUEST ENV BRANCH=$BRANCH - COPY $LOCAL_BASE_BENCH_FOLDER $BENCH_FOLDER/ - COPY +bench-aggregate/bench $BENCH_FOLDER + COPY +pack-base-benchmark/bench $BENCH_FOLDER/ + COPY +bench-aggregate/bench $BENCH_FOLDER/ RUN echo "Bench folder contents $(ls $BENCH_FOLDER)" RUN --secret AZTEC_BOT_COMMENTER_GITHUB_TOKEN \ [ -f $BENCH_FOLDER/benchmark.json ] \ From 219a9c9202f5cdad950d5259b28c6dbbcb0ee782 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 15 May 2024 15:22:57 -0300 Subject: [PATCH 4/9] Fix target syntax --- yarn-project/scripts/Earthfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index 164c4d47ea8..5e2d999e35c 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -55,7 +55,7 @@ pack-base-benchmark: ARG PULL_REQUEST ARG BRANCH ARG LOCAL_BENCH_FOLDER - FROM +scratch + FROM scratch LET BENCH_FOLDER=/usr/var/bench COPY --if-exists $LOCAL_BENCH_FOLDER $BENCH_FOLDER RUN mkdir -p $BENCH_FOLDER From 6cb7841f3b546f0be80dc670decb9e51fd6c0b97 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 16 May 2024 09:17:12 -0300 Subject: [PATCH 5/9] More fixes --- yarn-project/scripts/Earthfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index 5e2d999e35c..8ed85ca135c 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -55,10 +55,11 @@ pack-base-benchmark: ARG PULL_REQUEST ARG BRANCH ARG LOCAL_BENCH_FOLDER + LOCALLY + RUN mkdir -p $LOCAL_BENCH_FOLDER FROM scratch LET BENCH_FOLDER=/usr/var/bench COPY --if-exists $LOCAL_BENCH_FOLDER $BENCH_FOLDER - RUN mkdir -p $BENCH_FOLDER SAVE ARTIFACT $BENCH_FOLDER bench bench-comment: From 7742a307ebbc0f86f8146f731e3ae731e830e9ca Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 16 May 2024 10:49:57 -0300 Subject: [PATCH 6/9] Keep trying --- .github/workflows/ci.yml | 3 ++- yarn-project/scripts/Earthfile | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f232e2e61f0..3d0c7a3f305 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -179,7 +179,8 @@ jobs: scripts/logs/download_base_benchmark_from_s3.sh cd yarn-project/scripts # Package it into an earthly artifact - earthly-ci -P +pack-base-benchmark --LOCAL_BENCH_FOLDER="./bench" + mkdir -p ./bench + earthly-ci -P +pack-base-benchmark # Delete it so we don't accidentally bust the yarn-project+build cache rm -rf ./bench env: diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index 8ed85ca135c..1cd395bf2bc 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -54,10 +54,8 @@ pack-base-benchmark: ARG COMMIT_HASH ARG PULL_REQUEST ARG BRANCH - ARG LOCAL_BENCH_FOLDER - LOCALLY - RUN mkdir -p $LOCAL_BENCH_FOLDER FROM scratch + LET LOCAL_BENCH_FOLDER=./bench LET BENCH_FOLDER=/usr/var/bench COPY --if-exists $LOCAL_BENCH_FOLDER $BENCH_FOLDER SAVE ARTIFACT $BENCH_FOLDER bench From 283c93840cf7f5ff804ac009769ca8ba993ed834 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 16 May 2024 11:38:49 -0300 Subject: [PATCH 7/9] Keep trying --- yarn-project/scripts/Earthfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index 1cd395bf2bc..e63227cc321 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -57,7 +57,7 @@ pack-base-benchmark: FROM scratch LET LOCAL_BENCH_FOLDER=./bench LET BENCH_FOLDER=/usr/var/bench - COPY --if-exists $LOCAL_BENCH_FOLDER $BENCH_FOLDER + COPY $LOCAL_BENCH_FOLDER $BENCH_FOLDER SAVE ARTIFACT $BENCH_FOLDER bench bench-comment: From 2ac21899d5eae4f05a95139e72403c3f0075c04e Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 16 May 2024 18:54:02 -0300 Subject: [PATCH 8/9] Pack base benchmark outside yarn project altogether --- .github/workflows/ci.yml | 13 +++++-------- scripts/logs/Earthfile | 19 +++++++++++++++++++ yarn-project/scripts/Earthfile | 21 +-------------------- 3 files changed, 25 insertions(+), 28 deletions(-) create mode 100644 scripts/logs/Earthfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d0c7a3f305..45b15c30816 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -176,17 +176,14 @@ jobs: if: ${{ github.event_name == 'pull_request' }} run: | # Download the base benchmark locally (requires AWS creds and .git history) - scripts/logs/download_base_benchmark_from_s3.sh - cd yarn-project/scripts - # Package it into an earthly artifact - mkdir -p ./bench - earthly-ci -P +pack-base-benchmark - # Delete it so we don't accidentally bust the yarn-project+build cache - rm -rf ./bench + mkdir -p $BENCH_FOLDER + ./scripts/logs/download_base_benchmark_from_s3.sh + # Package it into an earthly artifact to read from bench-comment + earthly-ci -P ./scripts/logs+pack-base-benchmark --LOCAL_BENCH_FOLDER=$BENCH_FOLDER env: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - BENCH_FOLDER: "./yarn-project/scripts/bench" + BENCH_FOLDER: "./scripts/logs/tmp/bench" PULL_REQUEST: "${{ github.event.pull_request.number }}" - name: "Generate summary comment if pull request" if: ${{ github.event_name == 'pull_request' }} diff --git a/scripts/logs/Earthfile b/scripts/logs/Earthfile new file mode 100644 index 00000000000..1e073bcad37 --- /dev/null +++ b/scripts/logs/Earthfile @@ -0,0 +1,19 @@ +VERSION 0.8 +FROM node:18.19.0 + +pack-base-benchmark: + # Copies the base benchmark (ie the master run) into a container and packs it as an artifact, + # so it can be consumed from bench-comment. Note that we need to download base-benchmark + # outside of this target beforehand. We cannot run it within an Earthly container because it needs + # access to the .git folder, and we cannot run it with a LOCALLY statement because Earthly does + # not support secrets when running locally (and we need) the AWS access keys to access S3. + # We also cannot COPY the local file directly from bench-comment, since the file must be present + # within the build context so we can copy it (ie within yarn-project/scripts), and that invalidates + # the cache of yarn-project+build since it does a `COPY . .`, and we cannot add the bench file to + # earthlyignore or we would not be able to copy it from anywhere. So we need to place this target + # outside yarn-project altogether, since that folder should not be modified. + ARG LOCAL_BENCH_FOLDER + FROM scratch + LET BENCH_FOLDER=/usr/var/bench + COPY $LOCAL_BENCH_FOLDER $BENCH_FOLDER + SAVE ARTIFACT $BENCH_FOLDER bench diff --git a/yarn-project/scripts/Earthfile b/yarn-project/scripts/Earthfile index e63227cc321..dc8d34361b4 100644 --- a/yarn-project/scripts/Earthfile +++ b/yarn-project/scripts/Earthfile @@ -41,25 +41,6 @@ bench-aggregate: || (echo "Not all log files from benchmark jobs found"; mkdir -p $BENCH_FOLDER) SAVE ARTIFACT $BENCH_FOLDER bench -pack-base-benchmark: - # Copies the base benchmark (ie the master run) into a container and packs it as an artifact, - # so it can be consumed from bench-comment. Note that we need to download base-benchmark - # outside of this target beforehand. We cannot run it within an Earthly container because it needs - # access to the .git folder, and we cannot run it with a LOCALLY statement because Earthly does - # not support secrets when running locally (and we need) the AWS access keys to access S3. - # We also cannot COPY the local file directly from bench-comment, since the file must be present - # within the build context so we can copy it (ie within yarn-project/scripts), and that invalidates - # the cache of yarn-project+build since it does a `COPY . .`, and we cannot add the bench file to - # earthlyignore or we would not be able to copy it from anywhere. - ARG COMMIT_HASH - ARG PULL_REQUEST - ARG BRANCH - FROM scratch - LET LOCAL_BENCH_FOLDER=./bench - LET BENCH_FOLDER=/usr/var/bench - COPY $LOCAL_BENCH_FOLDER $BENCH_FOLDER - SAVE ARTIFACT $BENCH_FOLDER bench - bench-comment: # Use the scripts image to run bench comment after loading the benchmark from bench-aggregate # and the base benchmark (ie the benches from master to compare to) from pack-base-benchmark. @@ -73,7 +54,7 @@ bench-comment: ENV PULL_REQUEST=$PULL_REQUEST ENV PR_NUMBER=$PULL_REQUEST ENV BRANCH=$BRANCH - COPY +pack-base-benchmark/bench $BENCH_FOLDER/ + COPY ../../scripts/logs+pack-base-benchmark/bench $BENCH_FOLDER/ COPY +bench-aggregate/bench $BENCH_FOLDER/ RUN echo "Bench folder contents $(ls $BENCH_FOLDER)" RUN --secret AZTEC_BOT_COMMENTER_GITHUB_TOKEN \ From 76e8e34507ac0fa06eea732c7c37cc6e38a74f0f Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Fri, 17 May 2024 11:26:02 -0300 Subject: [PATCH 9/9] Paths are relative from earthfile, not workdir --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 45b15c30816..cd0fa22467f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -179,7 +179,7 @@ jobs: mkdir -p $BENCH_FOLDER ./scripts/logs/download_base_benchmark_from_s3.sh # Package it into an earthly artifact to read from bench-comment - earthly-ci -P ./scripts/logs+pack-base-benchmark --LOCAL_BENCH_FOLDER=$BENCH_FOLDER + earthly-ci -P ./scripts/logs+pack-base-benchmark --LOCAL_BENCH_FOLDER=./tmp/bench env: AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}