From 17a737a7527699f9c7709d697fe291dd3e4652b1 Mon Sep 17 00:00:00 2001 From: Moritz Kiefer Date: Tue, 22 Jun 2021 16:06:30 +0200 Subject: [PATCH] Simplify speedy benchmark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR drops two things: 1. The check that the benchmark hasn’t been modified. This hasn’t ever been useful and it keeps being annoying. 2. It stops the comparison against the old version and instead just benchmarks the current version. We really only care about the day to day changes. Comparing against an arbitrary year old version has lost all meaning at this point. changelog_begin changelog_end --- azure-pipelines.yml | 24 ---- ci/cron/daily-compat.yml | 20 +-- ci/cron/perf/BazelExclusiveSandboxing.patch | 15 --- ci/cron/perf/CollectAuthority.scala.patch | 135 -------------------- ci/cron/perf/bench.sh | 23 ++++ ci/cron/perf/compare.sh | 38 ------ ci/cron/perf/test_sha | 1 - 7 files changed, 29 insertions(+), 227 deletions(-) delete mode 100644 ci/cron/perf/BazelExclusiveSandboxing.patch delete mode 100644 ci/cron/perf/CollectAuthority.scala.patch create mode 100755 ci/cron/perf/bench.sh delete mode 100755 ci/cron/perf/compare.sh delete mode 100644 ci/cron/perf/test_sha diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 81e63c73a14c..bd7a4d325bbb 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -19,30 +19,6 @@ pr: none jobs: - template: ci/build.yml -- job: check_perf_test - pool: - name: ubuntu_20_04 - demands: assignment -equals default - steps: - - bash: | - TEST_SHA=$(cat ci/cron/perf/test_sha) - LAST_CHANGES=$(git log -n1 --format=%H daml-lf/scenario-interpreter/src/perf) - CURRENT_SHA=$(git rev-parse HEAD) - if [ "$TEST_SHA" != "$LAST_CHANGES" ]; then - if [ "$LAST_CHANGES" = "$CURRENT_SHA" ]; then - curl -XPOST \ - -i \ - -H 'Content-Type: application/json' \ - --data "{\"text\":\" Perf tests seem to have changed. Please manually check:\n\`\`\`\ngit diff $TEST_SHA $LAST_CHANGES -- daml-lf/scenario-interpreter/src/perf\n\`\`\`\nand update accordingly. If the change is benign, update \`ci/cron/perf/test_sha\` to \`$LAST_CHANGES\`. With no intervention, you will no longer get performance reports.\"}" \ - $(Slack.ci-failures-daml) - else - echo "Changes detected, but not from this commit." - fi - else - echo "No change detected." - fi - displayName: check perf changes - - job: release dependsOn: [ "check_for_release", "Linux", "macOS", "Windows" ] condition: and(succeeded(), diff --git a/ci/cron/daily-compat.yml b/ci/cron/daily-compat.yml index 1dd988b965a8..384eebf80d55 100644 --- a/ci/cron/daily-compat.yml +++ b/ci/cron/daily-compat.yml @@ -90,25 +90,17 @@ jobs: eval "$(dev-env/bin/dade assist)" source $(bash_lib) - BASELINE="cebc26af88efef4a7c81c62b0c14353f829b755e" - TEST_SHA=$(cat ci/cron/perf/test_sha) OUT="$(Build.StagingDirectory)/perf-results.json" START=$(date -u +%Y%m%d_%H%M%SZ) - if git diff --exit-code $TEST_SHA -- daml-lf/scenario-interpreter/src/perf >&2; then - # no changes, all good - ci/cron/perf/compare.sh $BASELINE > "$OUT" - cat "$OUT" - else - # the tests have changed, we need to figure out what to do with - # the baseline. - echo "Baseline no longer valid, needs manual correction." > "$OUT" - fi + ci/cron/perf/bench.sh > "$OUT" + + cat $OUT gcs "$GCRED" cp "$OUT" gs://daml-data/perf/speedy/$START.json - setvar speedup "$(jq -r '.speedup' "$OUT")" + setvar speedy_perf "$(jq -r '.current-perf' "$OUT")" displayName: measure perf name: out @@ -329,7 +321,7 @@ jobs: compatibility: $[ dependencies.compatibility.result ] compatibility_windows: $[ dependencies.compatibility_windows.result ] perf_speedy: $[ dependencies.perf_speedy.result ] - speedup: $[ dependencies.perf_speedy.outputs['out.speedup'] ] + speedy_perf: $[ dependencies.perf_speedy.outputs['out.speedy_perf'] ] perf_http_json: $[ dependencies.perf_http_json.result ] check_releases: $[ dependencies.check_releases.result ] blackduck_scan: $[ dependencies.blackduck_scan.result ] @@ -355,7 +347,7 @@ jobs: && "$(check_releases)" == "Succeeded" && ("$(blackduck_scan)" == "Succeeded" || "$(blackduck_scan)" == "Skipped") && ("$(run_notices_pr_build)" == "Succeeded" || "$(run_notices_pr_build)" == "Skipped") ]]; then - tell_slack "Daily tests passed: $COMMIT_LINK (speedup: $(speedup))." "$(Slack.ci-failures-daml)" + tell_slack "Daily tests passed: $COMMIT_LINK (speedy_perf: $(speedy_perf))." "$(Slack.ci-failures-daml)" else tell_slack "Daily tests failed: $COMMIT_LINK." "$(Slack.ci-failures-daml)" fi diff --git a/ci/cron/perf/BazelExclusiveSandboxing.patch b/ci/cron/perf/BazelExclusiveSandboxing.patch deleted file mode 100644 index 372a2bee36af..000000000000 --- a/ci/cron/perf/BazelExclusiveSandboxing.patch +++ /dev/null @@ -1,15 +0,0 @@ -diff --git a/nix/nixpkgs.nix b/nix/nixpkgs.nix -index 8c7eb189783..e0b7e1e5534 100644 ---- a/nix/nixpkgs.nix -+++ b/nix/nixpkgs.nix -@@ -38,8 +38,8 @@ let - # for the only change that actually affects the code in this patch. The rest is tests - # and/or documentation. - (pkgs.fetchurl { -- url = "https://patch-diff.githubusercontent.com/raw/bazelbuild/bazel/pull/8983.patch"; -- sha256 = "1j25bycn9q7536ab3ln6yi6zpzv2b25fwdyxbgnalkpl2dz9idb7"; -+ url = "https://github.com/bazelbuild/bazel/commit/0f0a0d58725603cf2f1c175963360b525718a195.patch"; -+ sha256 = "1h9sqlcz3nkpppkbk0rilwf7ivhxp9iwircfpm1mbbjwq2pwywqa"; - }) - ]; - }); diff --git a/ci/cron/perf/CollectAuthority.scala.patch b/ci/cron/perf/CollectAuthority.scala.patch deleted file mode 100644 index 74cd1e24afc8..000000000000 --- a/ci/cron/perf/CollectAuthority.scala.patch +++ /dev/null @@ -1,135 +0,0 @@ -diff --git a/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala b/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala -index 76689ce45..c723418a8 100644 ---- a/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala -+++ b/daml-lf/scenario-interpreter/src/perf/benches/scala/com/digitalasset/daml/lf/speedy/perf/CollectAuthority.scala -@@ -10,7 +10,11 @@ import com.daml.lf.archive.{Decode, UniversalArchiveReader} - import com.daml.lf.data._ - import com.daml.lf.data.Ref._ - import com.daml.lf.language.Ast._ --import com.daml.lf.speedy.Pretty._ -+import com.daml.lf.speedy.SResult._ -+import com.daml.lf.transaction.Transaction.Value -+import com.daml.lf.types.Ledger -+import com.daml.lf.types.Ledger._ -+import com.daml.lf.value.Value.{AbsoluteContractId, ContractInst} - import java.io.File - import java.util.concurrent.TimeUnit - import org.openjdk.jmh.annotations._ -@@ -42,23 +46,90 @@ class CollectAuthorityState { - Some(seeding())) - .fold(err => sys.error(err.toString), identity) - expr = EVal(Identifier(packages.main._1, QualifiedName.assertFromString(scenario))) -- // NOTE(MH): We run the machine once to initialize all data that is shared -- // between runs. -- val steps1 = run() -+ setup() - } - -- def run(): Int = { -+ def run(): Unit = { - val machine = buildMachine(expr) -- ScenarioRunner(machine).run() match { -- case Left((err, _)) => sys.error(prettyError(err, machine.ptx).render(80)) -- case Right((_, steps, _)) => steps -+ var step = 0 -+ while(!machine.isFinal) { -+ machine.step() match { -+ case SResultScenarioGetParty(_, callback) => step += 1; callback(cachedParty(step)) -+ case SResultScenarioCommit(_, _, _, callback) => step += 1; callback(cachedCommit(step)) -+ case SResultNeedContract(_, _, _, _, callback) => step += 1; callback(cachedContract(step)) -+ case SResultContinue => () -+ case r => crash("bench run: unexpected result from speedy") -+ } - } - } -+ -+ private var cachedParty: Map[Int, Party] = Map() -+ private var cachedCommit: Map[Int, SValue] = Map() -+ private var cachedContract: Map[Int, ContractInst[Value[AbsoluteContractId]]] = Map() -+ -+ def setup(): Unit = { -+ cachedParty = Map() -+ cachedCommit = Map() -+ cachedContract = Map() -+ val machine = buildMachine(expr) -+ var step = 0 -+ var ledger: Ledger = Ledger.initialLedger(Time.Timestamp.Epoch) -+ while (!machine.isFinal) { -+ machine.step() match { -+ case SResultContinue => () -+ case SResultScenarioGetParty(partyText, callback) => -+ step += 1 -+ Party.fromString(partyText) match { -+ case Right(res) => -+ cachedParty = cachedParty + (step -> res) -+ callback(res) -+ case Left(msg) => -+ crash(s"Party.fromString failed: $msg") -+ } -+ case SResultScenarioCommit(value, tx, committers, callback) => -+ step += 1 -+ Ledger.commitTransaction( -+ committers.head, -+ ledger.currentTime, -+ machine.commitLocation, -+ tx, -+ ledger -+ ) match { -+ case Left(fas) => crash(s"commitTransaction failed: $fas") -+ case Right(result) => -+ ledger = result.newLedger -+ val res = -+ value -+ .mapContractId( -+ coid => -+ Ledger -+ .contractIdToAbsoluteContractId(result.transactionId, coid)) -+ cachedCommit = cachedCommit + (step -> res) -+ callback(res) -+ } -+ case SResultNeedContract(acoid, _, committers, _, callback) => -+ step += 1 -+ val effectiveAt = ledger.currentTime -+ ledger.lookupGlobalContract(ParticipantView(committers.head), effectiveAt, acoid) match { -+ case LookupOk(_, result) => -+ cachedContract = cachedContract + (step -> result) -+ callback(result) -+ case x => -+ crash(s"lookupGlobalContract failed: $x") -+ } -+ case _ => -+ crash("setup run: unexpected result from speedy") -+ } -+ } -+ } -+ -+ def crash(reason: String) = -+ throw new RuntimeException(s"CollectAuthority: $reason") - } - - class CollectAuthority { - @Benchmark @BenchmarkMode(Array(Mode.AverageTime)) @OutputTimeUnit(TimeUnit.MILLISECONDS) -- def bench(state: CollectAuthorityState): Int = { -+ def bench(state: CollectAuthorityState): Unit = { - state.run() - } - } -diff --git a/stack-snapshot.yaml b/stack-snapshot.yaml -index 01ab7e789..c2e09093d 100644 ---- a/stack-snapshot.yaml -+++ b/stack-snapshot.yaml -@@ -3,10 +3,10 @@ - - resolver: lts-14.1 - packages: -- - archive: http://digitalassetsdk.bintray.com/ghc-lib/ghc-lib-parser-8.8.1.20200225.tar.gz -- sha256: "31b6f1fc15d257e66d3264e62e229203087a2e799a9638f6d5ff6cc43ec0b4a4" -- - archive: http://digitalassetsdk.bintray.com/ghc-lib/ghc-lib-8.8.1.20200225.tar.gz -- sha256: "2cd22db09fdcb9bb121bd6f85024aba437ca2796ebe16453d15cbf2cb192023c" -+ - archive: https://daml-binaries.da-ext.net/da-ghc-lib/ghc-lib-48295232a0bebf0bf80b90447d0f5890.tar.gz -+ sha256: "32ef1e2aebdd473681e38fbbf4747b88a1e03d00ea90adb9917b613659acad9f" -+ - archive: https://daml-binaries.da-ext.net/da-ghc-lib/ghc-lib-parser-48295232a0bebf0bf80b90447d0f5890.tar.gz -+ sha256: "dbaaca05c677794261e102f1a7d6a4409d368a47feb214b6378fa1169d269ec3" - - github: digital-asset/hlint - commit: "3e78bce69749b22a80fec1e8eb853cc0c100c18e" - sha256: "cf39f2b378485afc77ffdad4dbb057d5d9b4dfc5a38c76ddc44e920e537fb0fa" diff --git a/ci/cron/perf/bench.sh b/ci/cron/perf/bench.sh new file mode 100755 index 000000000000..a8b1dce746c9 --- /dev/null +++ b/ci/cron/perf/bench.sh @@ -0,0 +1,23 @@ +# Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +#!/usr/bin/env bash + +set -euox pipefail + +measure() { + local out=$(mktemp -d)/out.json + bazel run daml-lf/scenario-interpreter:scenario-perf -- -rf json -rff $out >&2 + cat $out | jq '.[0].primaryMetric.score' +} + +main() { + local current=$(git rev-parse HEAD) + + local current_perf=$(measure) + if [ "" = "$current_perf" ]; then exit 1; fi + + echo '{"current-perf": '$current_perf', "current-sha": "'$current'"}' +} + +main diff --git a/ci/cron/perf/compare.sh b/ci/cron/perf/compare.sh deleted file mode 100755 index 9f5e655af4d3..000000000000 --- a/ci/cron/perf/compare.sh +++ /dev/null @@ -1,38 +0,0 @@ -# Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -#!/usr/bin/env bash - -set -euox pipefail - -BASELINE=$1 - -measure() { - local out=$(mktemp -d)/out.json - bazel run daml-lf/scenario-interpreter:scenario-perf -- -rf json -rff $out >&2 - cat $out | jq '.[0].primaryMetric.score' -} - -main() { - local current=$(git rev-parse HEAD) - - git checkout $BASELINE >&2 - git show ${current}:ci/cron/perf/CollectAuthority.scala.patch | git apply - git show ${current}:ci/cron/perf/BazelExclusiveSandboxing.patch | git apply - local baseline_perf=$(measure) - if [ "" = "$baseline_perf" ]; then exit 1; fi - - # undo patch - git reset --hard >&2 - git checkout $current >&2 - local current_perf=$(measure) - if [ "" = "$current_perf" ]; then exit 1; fi - - local speedup=$(printf "%.2f" $(echo "$baseline_perf / $current_perf" | bc -l)) - local progress_5x=$(printf "%05.2f%%" $(echo "100 * l($speedup) / l(5)" | bc -l)) - local progress_10x=$(printf "%05.2f%%" $(echo "100 * l($speedup) / l(10)" | bc -l)) - - echo '{"current-perf": '$current_perf', "baseline-perf": '$baseline_perf', "speedup": "'$speedup'x", "progress_towards_5x": "'$progress_5x'", "progress_towards_10x": "'$progress_10x'", "current-sha": "'$current'", "baseline-sha": "'$BASELINE'"}' -} - -main diff --git a/ci/cron/perf/test_sha b/ci/cron/perf/test_sha deleted file mode 100644 index 1aab6836ba47..000000000000 --- a/ci/cron/perf/test_sha +++ /dev/null @@ -1 +0,0 @@ -febca5d62d431916f958b207e6115d0eb14c52e7