Benchmark fixes (#1971)

# Goal The goal of this PR is to fix an issue with running benchmarks and update our benchmarks with some tests. Part of #1853 ## Issues - When I added the Proxy pallet, the copied weight file worked, but the generated version didn't. - Then when fixing that, the benchmark runners had issues. So updated the benchmark workflow by removing it from the commit commenting system to a manual run. Using that will also automatically trigger verify CI actions. (GitHub actions only run on Manually triggered actions to prevent loops) # Discussion - The additional changes to the templates is from Polkadot-SDK - Actual change is small, as most of this are the generated weights code - Fix issue with the proxy_pallet weights reference - Update the benchmark running system to not use special Github tokens
frequency-chain · May 22, 2024 · 0db0a79 · 0db0a79
1 parent 0d8306d
commit 0db0a79
Show file tree

Hide file tree

Showing 36 changed files with 3,241 additions and 1,215 deletions.
diff --git a/.github/workflows/benchmarks-run.yml b/.github/workflows/benchmarks-run.yml
@@ -1,5 +1,6 @@
 # yaml-language-server: $schema=https://json.schemastore.org/github-workflow.json
 name: Benchmarks Run
+run-name: Benchmarks Run for ${{github.event.inputs.branch}}
 concurrency:
   group: ${{github.workflow}}-${{github.ref}}
   cancel-in-progress: true
@@ -32,22 +33,6 @@ jobs:
         uses: actions/checkout@v4
         with:
           ref: ${{github.event.inputs.branch}}
-      - name: Get Current Job Log URL
-        id: jobs
-        uses: Tiryoh/gha-jobid-action@c1d1cf7334b70c29374ae382b91053674c8049a2
-        with:
-          github_token: ${{ github.token }}
-          job_name: "Run Benchmarks"
-      - name: Set PR Commit Status to Pending
-        uses: ouzi-dev/commit-status-updater@219d3f932547cad092e384c7a36bf4d963739c35
-        with:
-          name: "GithubActions - Run Benchmarks"
-          status: "pending"
-          addHoldComment: "true"
-          pendingComment: >-
-            :hourglass_flowing_sand: [Running benchmarks](${{steps.jobs.outputs.html_url}})
-            and calculating weights. DO NOT MERGE! A new commit will be added upon completion...
-          failComment: "ERROR: Failed to set commit to pending status!"
       - name: Install Required Packages
         run: |
           sudo apt-get update
@@ -80,17 +65,5 @@ jobs:
         id: commit-updated-weights
         uses: stefanzweifel/git-auto-commit-action@8756aa072ef5b4a080af5dc8fef36c5d586e521d
         with:
-          commit_message: "Update weights for #${{github.event.inputs.branch}}"
+          commit_message: "Update weights"
           file_pattern: "pallets/**/*.rs runtime/common/src/weights/*"
-
-      - name: Set Commit Status to Success
-        uses: ouzi-dev/commit-status-updater@219d3f932547cad092e384c7a36bf4d963739c35
-        with:
-          name: "GithubActions - Run Benchmarks"
-          status: "success"
-          addHoldComment: "true"
-          successComment: >-
-            :white_check_mark: [Finished running benchmarks](${{steps.jobs.outputs.html_url}}).
-            Updated weights have been committed to this branch in
-            commit ${{steps.commit-updated-weights.outputs.commit_hash}}.
-          failComment: "ERROR: Failed to set commit to success status!"
diff --git a/.github/workflows/create-issue-dependabot-alert-pr.yml b/.github/workflows/create-issue-dependabot-alert-pr.yml
@@ -11,8 +11,10 @@ on:
 jobs:
   create-issue:
     runs-on: ubuntu-22.04
+    permissions:
+      contents: write
     env:
-      GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
+      GITHUB_TOKEN: ${{ github.token }}
       PR_TITLE: ${{github.event.pull_request.title}}
       PR_NUMBER: ${{github.event.pull_request.number}}
       PR_URL: ${{github.event.pull_request.url}}

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -19,6 +19,7 @@ on:
 
 permissions:
   contents: write
+  statuses: write
 
 env:
   NEW_RELEASE_TAG_FROM_UI: ${{github.event.inputs.release-version}}
@@ -94,8 +95,8 @@ jobs:
         if: steps.is-full-release.outputs.is-full-release == 'true'
         # Match installation steps to CI base docker image
         run: |
-          curl https://sh.rustup.rs -sSf | bash -s -- -y
-          echo "PATH=$HOME/.cargo/bin:$PATH" >> $GITHUB_ENV
+          curl https://sh.rustup.rs -sSf | HOME=`pwd` bash -s -- -y
+          echo "PATH=`pwd`/.cargo/bin:$PATH" >> $GITHUB_ENV
       - name: Update Weights for All Pallets
         if: steps.is-full-release.outputs.is-full-release == 'true'
         run: |
@@ -108,9 +109,7 @@ jobs:
         uses: stefanzweifel/git-auto-commit-action@8756aa072ef5b4a080af5dc8fef36c5d586e521d
         with:
           commit_message: "Update weights for release ${{env.NEW_RELEASE_TAG}}"
-          commit_user_name: Frequency CI [bot]
-          commit_user_email: do-not-reply@users.noreply.github.com
-          commit_author: Frequency CI [bot] <do-not-reply@users.noreply.github.com>
+          file_pattern: "pallets/**/*.rs runtime/common/src/weights/*"
 
   version-code:
     needs: run-all-benchmarks
@@ -136,9 +135,6 @@ jobs:
         uses: stefanzweifel/git-auto-commit-action@8756aa072ef5b4a080af5dc8fef36c5d586e521d
         with:
           commit_message: "Update versions for release ${{env.NEW_RELEASE_TAG}}"
-          commit_user_name: Frequency CI [bot]
-          commit_user_email: do-not-reply@users.noreply.github.com
-          commit_author: Frequency CI [bot] <do-not-reply@users.noreply.github.com>
       - name: Update Release Version Tag
         uses: rickstaa/action-create-tag@88dbf7ff6fe2405f8e8f6c6fdfd78829bc631f83
         with:
@@ -651,6 +647,8 @@ jobs:
     name: Release Built Artifacts
     runs-on: ubuntu-22.04
     container: ghcr.io/frequency-chain/frequency/ci-base-image:1.3.1
+    permissions:
+      contents: write
     steps:
       - name: Check Out Repo
         uses: actions/checkout@v4
@@ -682,7 +680,7 @@ jobs:
         id: build-changelog
         uses: mikepenz/release-changelog-builder-action@6fd5cc6eaf7567dbd0f9666061215bb476f012fc
         env:
-          GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
+          GITHUB_TOKEN: ${{ github.token }}
         with:
           fromTag: ${{env.PREVIOUS_RELEASE_TAG}}
           toTag: ${{env.NEW_RELEASE_TAG}}

diff --git a/.github/workflows/verify-pr-commit.yml b/.github/workflows/verify-pr-commit.yml
@@ -56,18 +56,20 @@ jobs:
   clear-metadata-labels:
     name: Clear Metadata Labels
     runs-on: ubuntu-22.04
+    permissions:
+      pull-requests: write
     steps:
       - name: Clear Metadata Changed Label
         if: contains(github.event.pull_request.labels.*.name, env.PR_LABEL_METADATA_CHANGED)
         uses: RobinJesba/GitHub-Labeler-Action@2f69380bbf2ee60b2f0893ef0f40582c9a34a64d
         with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ github.token }}
           LABELS_TO_REMOVE: ${{env.PR_LABEL_METADATA_CHANGED}}
       - name: Clear Metadata Version Not Incremented Label
         if: contains(github.event.pull_request.labels.*.name, env.PR_LABEL_METADATA_VERSION_NOT_INCREMENTED)
         uses: RobinJesba/GitHub-Labeler-Action@2f69380bbf2ee60b2f0893ef0f40582c9a34a64d
         with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ github.token }}
           LABELS_TO_REMOVE: ${{env.PR_LABEL_METADATA_VERSION_NOT_INCREMENTED}}
 
   # Workaround to handle skipped required check inside matrix
@@ -541,6 +543,8 @@ jobs:
     needs: build-binaries
     name: Check Metadata and Spec Version
     runs-on: ubuntu-22.04
+    permissions:
+      pull-requests: write
     steps:
       - name: Check Out Repo
         uses: actions/checkout@v4
@@ -583,7 +587,7 @@ jobs:
         if: steps.compare-metadata.outputs.metadata_matches != 'true'
         uses: RobinJesba/GitHub-Labeler-Action@2f69380bbf2ee60b2f0893ef0f40582c9a34a64d
         with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ github.token }}
           LABELS_TO_ADD: ${{env.PR_LABEL_METADATA_CHANGED}}
       - name: Check Spec Version
         if: steps.compare-metadata.outputs.metadata_matches != 'true'
@@ -603,7 +607,7 @@ jobs:
             (steps.check-spec-version.outputs.metadata_version_incremented != 'true')
         uses: RobinJesba/GitHub-Labeler-Action@2f69380bbf2ee60b2f0893ef0f40582c9a34a64d
         with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_TOKEN: ${{ github.token }}
           LABELS_TO_ADD: ${{env.PR_LABEL_METADATA_VERSION_NOT_INCREMENTED}}
       - name: Fail CI
         if: |
@@ -750,114 +754,3 @@ jobs:
           echo "Actual genesis state: $actual_genesis_state"
           [ $actual_genesis_state = $expected_genesis_state ] || \
             (echo "ERROR: The actual genesis state does not match the expected" && exit 1)
-
-  should-run-benchmarks:
-    if: github.repository == 'frequency-chain/frequency'
-    name: Run Benchmarks?
-    runs-on: ubuntu-22.04
-    container: ghcr.io/frequency-chain/frequency/ci-base-image:1.3.1
-    outputs:
-      should_run_benchmarks: ${{steps.run-benchmarks.outputs.run}}
-      pallets: ${{steps.run-benchmarks.outputs.pallets}}
-    steps:
-      - name: Check Out Repo
-        uses: actions/checkout@v4
-        with:
-          ref: ${{github.event.pull_request.head.sha}}
-      - name: Run Benchmarks?
-        id: run-benchmarks
-        shell: bash
-        run: |
-          git config --global --add safe.directory /__w/frequency/frequency
-          commit_message=$(git show -s --format=%s)
-          echo "commit_message: $commit_message"
-          regex='\[run-benchmarks (.*)\]$'
-          if [[ $commit_message =~ $regex ]]; then
-            echo "Yes, need to run benchmarks."
-            pallets="${BASH_REMATCH[1]}"
-            echo "Pallets: $pallets"
-            echo "run=true" >> $GITHUB_OUTPUT
-            echo "pallets=$pallets" >> $GITHUB_OUTPUT
-          else
-            echo "No, do not need to run benchmarks."
-            echo "run=false" >> $GITHUB_OUTPUT
-          fi
-
-  run-benchmarks:
-    if: github.repository == 'frequency-chain/frequency' &&
-      needs.should-run-benchmarks.outputs.should_run_benchmarks == 'true'
-    needs: should-run-benchmarks
-    name: Run Benchmarks
-    runs-on: [self-hosted, Linux, X64, benchmark]
-    steps:
-      - name: Print Info
-        run: |
-          echo "Running benchmarks..."
-          echo "Pallets: ${{needs.should-run-benchmarks.outputs.pallets}}"
-      - name: Check Out Repo
-        uses: actions/checkout@v4
-        with:
-          token: ${{secrets.GHA_GIT_COMMIT || github.token}}
-          ref: ${{github.event.pull_request.head.sha}}
-      - name: Get Current Job Log URL
-        id: jobs
-        uses: Tiryoh/gha-jobid-action@7528cee9716384209bafcbd000d6689e851c5dda
-        with:
-          github_token: ${{secrets.GITHUB_TOKEN}}
-          job_name: "Run Benchmarks"
-      - name: Set PR Commit Status to Pending
-        uses: ouzi-dev/commit-status-updater@219d3f932547cad092e384c7a36bf4d963739c35
-        with:
-          name: "GithubActions - Run Benchmarks"
-          status: "pending"
-          addHoldComment: "true"
-          pendingComment: >-
-            :hourglass_flowing_sand: [Running benchmarks](${{steps.jobs.outputs.html_url}})
-            and calculating weights. DO NOT MERGE! A new commit will be added upon completion...
-          failComment: "ERROR: Failed to set PR commit to pending status!"
-      - name: Install Required Packages
-        run: |
-          sudo apt-get update
-          sudo apt install -y protobuf-compiler libclang-dev clang cmake
-      - name: Install Rust Toolchain
-        # Match installation steps to CI base docker image
-        run: |
-          curl https://sh.rustup.rs -sSf | bash -s -- -y
-          echo "PATH=$HOME/.cargo/bin:$PATH" >> $GITHUB_ENV
-      - name: Update Weights
-        run: |
-          rustup show
-          pallets_str="${{needs.should-run-benchmarks.outputs.pallets}}"
-          echo "Pallets: $pallets_str"
-          if [ -z "${pallets_str}" -o $pallets_str = 'all' ]; then
-            echo "Running benchmarks for all pallets..."
-            make benchmarks
-          else
-            IFS=',' read -r -a pallets <<< "$pallets_str"
-              echo "Running benchmarks for pallets: ${pallets[*]}..."
-              make benchmarks-multi PALLETS="${pallets[*]}"
-              echo "Finished benchmarks for pallets: ${pallets[*]}."
-          fi
-      - name: Print Updated Weights
-        run: |
-          git status
-          git diff
-      - name: Commit Updated Weights
-        id: commit-updated-weights
-        uses: stefanzweifel/git-auto-commit-action@8756aa072ef5b4a080af5dc8fef36c5d586e521d
-        with:
-          commit_message: "Update weights for PR #${{github.event.pull_request.number}}"
-          commit_user_name: Frequency CI [bot]
-          commit_user_email: do-not-reply@users.noreply.github.com
-          commit_author: Frequency CI [bot] <do-not-reply@users.noreply.github.com>
-      - name: Set PR Commit Status to Success
-        uses: ouzi-dev/commit-status-updater@219d3f932547cad092e384c7a36bf4d963739c35
-        with:
-          name: "GithubActions - Run Benchmarks"
-          status: "success"
-          addHoldComment: "true"
-          successComment: >-
-            :white_check_mark: [Finished running benchmarks](${{steps.jobs.outputs.html_url}}).
-            Updated weights have been committed to this PR branch in
-            commit ${{steps.commit-updated-weights.outputs.commit_hash}}.
-          failComment: "ERROR: Failed to set PR commit to success status!"
diff --git a/.maintain/frame-weight-template.hbs b/.maintain/frame-weight-template.hbs
@@ -1,27 +1,11 @@
-// This file is part of Substrate.
-
-// Copyright (C) 2022 Parity Technologies (UK) Ltd.
-// SPDX-License-Identifier: Apache-2.0
-
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//! Autogenerated weights for {{pallet}}
+{{header}}
+//! Autogenerated weights for `{{pallet}}`
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}}
 //! DATE: {{date}}, STEPS: `{{cmd.steps}}`, REPEAT: `{{cmd.repeat}}`, LOW RANGE: `{{cmd.lowest_range_values}}`, HIGH RANGE: `{{cmd.highest_range_values}}`
 //! WORST CASE MAP SIZE: `{{cmd.worst_case_map_values}}`
 //! HOSTNAME: `{{hostname}}`, CPU: `{{cpuname}}`
-//! EXECUTION: {{cmd.execution}}, WASM-EXECUTION: {{cmd.wasm_execution}}, CHAIN: {{cmd.chain}}, DB CACHE: {{cmd.db_cache}}
+//! WASM-EXECUTION: `{{cmd.wasm_execution}}`, CHAIN: `{{cmd.chain}}`, DB CACHE: `{{cmd.db_cache}}`
 
 // Executed Command:
 {{#each args as |arg|}}
@@ -36,7 +20,7 @@
 use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};
 use core::marker::PhantomData;
 
-/// Weight functions needed for {{pallet}}.
+/// Weight functions needed for `{{pallet}}`.
 pub trait WeightInfo {
 	{{#each benchmarks as |benchmark|}}
 	fn {{benchmark.name~}}
@@ -47,7 +31,7 @@ pub trait WeightInfo {
 	{{/each}}
 }
 
-/// Weights for {{pallet}} using the Substrate node and recommended hardware.
+/// Weights for `{{pallet}}` using the Substrate node and recommended hardware.
 pub struct SubstrateWeight<T>(PhantomData<T>);
 {{#if (eq pallet "frame_system")}}
 impl<T: crate::Config> WeightInfo for SubstrateWeight<T> {
@@ -94,7 +78,7 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 	{{/each}}
 }
 
-// For backwards compatibility and tests
+// For backwards compatibility and tests.
 impl WeightInfo for () {
 	{{#each benchmarks as |benchmark|}}
 	{{#each benchmark.comments as |comment|}}
@@ -135,3 +119,43 @@ impl WeightInfo for () {
 	}
 	{{/each}}
 }
+
+
+#[cfg(test)]
+mod tests {
+  use frame_support::{traits::Get, weights::Weight, dispatch::DispatchClass};
+  use common_runtime::constants::{MAXIMUM_BLOCK_WEIGHT, NORMAL_DISPATCH_RATIO};
+  use common_runtime::weights::extrinsic_weights::ExtrinsicBaseWeight;
+
+  struct BlockWeights;
+  impl Get<frame_system::limits::BlockWeights> for BlockWeights {
+  	fn get() -> frame_system::limits::BlockWeights {
+  		frame_system::limits::BlockWeights::builder()
+  			.base_block(Weight::zero())
+  			.for_class(DispatchClass::all(), |weights| {
+  				weights.base_extrinsic = ExtrinsicBaseWeight::get().into();
+  			})
+  			.for_class(DispatchClass::non_mandatory(), |weights| {
+  				weights.max_total = Some(NORMAL_DISPATCH_RATIO * MAXIMUM_BLOCK_WEIGHT);
+  			})
+  			.build_or_panic()
+  	}
+  }
+
+	{{#each benchmarks as |benchmark|}}
+	{{#if (ne benchmark.base_calculated_proof_size "0")}}
+	#[test]
+	fn test_{{benchmark.name~}} () {
+		assert!(
+			BlockWeights::get()
+				.per_class
+				.get(frame_support::dispatch::DispatchClass::Normal)
+				.max_extrinsic
+				.unwrap_or_else(<Weight as sp_runtime::traits::Bounded>::max_value)
+				.proof_size()
+				> {{benchmark.base_calculated_proof_size}}
+		);
+	}
+	{{/if}}
+	{{/each}}
+}