Kwxm/bench compare markdown #3471

kwxm · 2021-07-02T04:08:58Z

Just testing.

Pre-submit checklist:

Branch
- Commit sequence broadly makes sense
- Key commits have useful messages
- Relevant tickets are mentioned in commit messages
- Formatting, materialized Nix files, PNG optimization, etc. are updated
PR
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested

Pre-merge checklist:

Someone approved it
Commits have useful messages
Review clarifications made it into the code
History is moderately tidy; or going to squash-merge

kwxm · 2021-07-02T04:09:18Z

/benchmark

kwxm · 2021-07-02T04:19:16Z

/benchmark

kwxm · 2021-07-02T05:18:27Z

@gilligan @michaelpj

There seems to be something strange going on here. I modified ci-plutus-benchmark.sh to print out the branches it was using:

PR_BRANCH_REF=$(git show-ref -s HEAD)
echo "### PR_BRANCH_REF=$PR_BRANCH_REF"

and then later

git checkout "$(git merge-base HEAD master)"
BASE_BRANCH_REF=$(git show-ref -s HEAD)
echo "### BASE_BRANCH_REF=$BASE_BRANCH_REF"

(the echos are added) and in the log it said

[ci-plutus-benchmark]: Processing benchmark comparison for PR 3471
### PR_BRANCH_REF=f37f9cca5662fede7c334dfbf839d836a2926eaa
...

ci-plutus-benchmark]: Running benchmark for PR branch ...
[ci-plutus-benchmark]: Switching branches ...
Previous HEAD position was 204a1cb80 Make branches more obvious
HEAD is now at f37f9cca5 Fix contract placeholders to work with LocalStorage. (#3401)
### BASE_BRANCH_REF=f37f9cca5662fede7c334dfbf839d836a2926eaa
[ci-plutus-benchmark]: Running benchmark for base branch

Now f37f9cc is this commit by Amyas from the 21st of June. I have no idea what that's doing there! I'm also not sure if that's the commit that's actually checked when it's running the tests.

kwxm · 2021-07-02T05:37:33Z

It seems to be doing the same thing with Michael's PR #3340 (see the log at https://buildkite.com/input-output-hk/plutus-benchmark/builds/32#5b90cecf-0926-412d-a7bf-6e672ff991d5).

[ci-plutus-benchmark]: Running benchmark for PR branch ...
[ci-plutus-benchmark]: Switching branches ...
Previous HEAD position was 655fe6060 SCP-2289: Try making a special "for evaluation" term type
HEAD is now at f37f9cca5 Fix contract placeholders to work with LocalStorage. (#3401)
[ci-plutus-benchmark]: Running benchmark for base branch ...
[ci-plutus-benchmark]: Comparing results ...

If it really is running the benchmarks in f37f9cc then that contains the old versions of the validation scripts (before @bezirg changed the flat encoding) which could have some bearing on the weird regression that I couldn't reproduce manually.

michaelpj · 2021-07-02T09:26:44Z

Ah, I know what's up! Is anything updating the checkout? If it has a very old version of master then git merge-base HEAD master will give you the wrong answer.

michaelpj · 2021-07-02T09:27:07Z

I don't know how that checkout is even created, though.

kwxm · 2021-07-02T09:35:29Z

I don't know how that checkout is even created, though.

Me neither. I have enough trouble with git on my own machine, so when it's being automatically run on a remote machine by a system I don't understand I'm lost.

Can we just get it to update master before it does anything else?

kwxm · 2021-07-05T13:10:40Z

Experimenting a bit with the comparison automation script.

/bench

michaelpj · 2021-07-05T13:11:16Z

Also note there are conflicts with master now.

kwxm · 2021-07-05T14:06:42Z

/benchmark

kwxm · 2021-07-05T18:18:02Z

The last lot failed because of a problem with builtins in the flat format. Let's try again.

/benchmark

kwxm · 2021-07-05T18:31:54Z

Hmmm. Getting lots of this in the output



Previous HEAD position was 84fb35a4c Merge branch 'master' into kwxm/bench-compare-markdown
--
  | Switched to branch 'master'
  | Your branch is up to date with 'origin/master'.
  | remote: Enumerating objects: 2427, done.
  | remote: Counting objects: 100% (2411/2411), done.
  | remote: Compressing objects: 100% (831/831), done.
  | remote: Total 2427 (delta 1494), reused 2303 (delta 1451), pack-reused 16
  | Receiving objects: 100% (2427/2427), 1.07 MiB \| 6.61 MiB/s, done.
  | Resolving deltas: 100% (1494/1494), completed with 446 local objects.
  | From github.com:input-output-hk/plutus
  | f37f9cca5..679db73c5  master                  -> origin/master
  | * [new branch]          SCP-2045-bis            -> origin/SCP-2045-bis
  | b29d7aae3..4136cfc67  actus-test-framework    -> origin/actus-test-framework
  | * [new branch]          benchmarking-ci         -> origin/benchmarking-ci

Then lots of this:



plutus-core/plutus-core/test/TypeSynthesis/Spec.hs \|    3 +-
--
  | plutus-core/plutus-ir/src/PlutusIR/Compiler.hs     \|    8 +-
  | .../plutus-ir/src/PlutusIR/Transform/DeadCode.hs   \|   10 +-
  | .../plutus-ir/src/PlutusIR/Transform/Inline.hs     \|   15 +-
  | .../plutus-ir/src/PlutusIR/Transform/Rename.hs     \|    6 +-
  | plutus-core/plutus-ir/test/TransformSpec.hs        \|    5 +-
  | .../plutus-ir/test/datatypes/listMatchEval.golden  \|    2 +-
  | .../plutus-ir/test/recursion/even3Eval.golden      \|    2 +-

Looks like it's replaying lots of changes when it does

git checkout master
git pull
git checkout "$PR_BRANCH_REF"

I have no idea if that's sensible or not.

kwxm · 2021-07-05T18:51:48Z

Got a variable name wrong the last time.

/benchmark

iohk-devops · 2021-07-05T19:02:46Z

Comparing benchmark results of '38d9354bde98e27e9756471aae062e7a56ab857e' (base) and '6eb2f00f5532013c8be24a8118ae280c12acee24' (PR)

Script	8d9354b	eb2f00f	Change
stablecoin/stablecoin_1-1	2.115 ms	2.117 ms	+0.1%
stablecoin/stablecoin_1-2	597.6 μs	596.9 μs	-0.1%
stablecoin/stablecoin_1-3	2.382 ms	2.375 ms	-0.3%
stablecoin/stablecoin_1-4	649.5 μs	649.3 μs	-0.0%
stablecoin/stablecoin_1-5	3.012 ms	2.999 ms	-0.4%
stablecoin/stablecoin_1-6	834.2 μs	832.5 μs	-0.2%
stablecoin/stablecoin_2-1	2.106 ms	2.099 ms	-0.3%
stablecoin/stablecoin_2-2	599.0 μs	596.2 μs	-0.5%
stablecoin/stablecoin_2-3	2.376 ms	2.370 ms	-0.3%
stablecoin/stablecoin_2-4	648.7 μs	647.1 μs	-0.2%

kwxm · 2021-07-05T19:07:19Z

/benchmark

iohk-devops · 2021-07-05T19:18:31Z

Comparing benchmark results of '38d9354bde98e27e9756471aae062e7a56ab857e' (base) and '6eb2f00f5532013c8be24a8118ae280c12acee24' (PR)

Script	8d9354b	eb2f00f	Change
stablecoin/stablecoin_1-1	2.101 ms	2.116 ms	+0.7%
stablecoin/stablecoin_1-2	596.0 μs	597.2 μs	+0.2%
stablecoin/stablecoin_1-3	2.380 ms	2.383 ms	+0.1%
stablecoin/stablecoin_1-4	648.1 μs	648.7 μs	+0.1%
stablecoin/stablecoin_1-5	3.003 ms	3.003 ms	0.0%
stablecoin/stablecoin_1-6	834.7 μs	833.3 μs	-0.2%
stablecoin/stablecoin_2-1	2.109 ms	2.102 ms	-0.3%
stablecoin/stablecoin_2-2	599.5 μs	597.1 μs	-0.4%
stablecoin/stablecoin_2-3	2.395 ms	2.376 ms	-0.8%
stablecoin/stablecoin_2-4	649.2 μs	648.0 μs	-0.2%

kwxm · 2021-07-05T20:25:29Z

/benchmark

iohk-devops · 2021-07-05T21:24:43Z

Comparing benchmark results of '38d9354bde98e27e9756471aae062e7a56ab857e' (base) and 'b8a4b0833c124b70a33d49cc0b3c100918d62d1f' (PR)

Script	`38d9354`	`b8a4b08`	Change
crowdfunding/crowdfunding-success-1	653.9 μs	652.6 μs	-0.2%
crowdfunding/crowdfunding-success-2	654.2 μs	651.3 μs	-0.4%
crowdfunding/crowdfunding-success-3	654.0 μs	652.3 μs	-0.3%
currency/currency-1	585.0 μs	582.5 μs	-0.4%
currency/currency-2	813.9 μs	814.0 μs	+0.0%
escrow/escrow-redeem_1-1	956.9 μs	960.2 μs	+0.3%
escrow/escrow-redeem_1-2	957.4 μs	958.5 μs	+0.1%
escrow/escrow-redeem_2-1	1.092 ms	1.095 ms	+0.3%
escrow/escrow-redeem_2-2	1.093 ms	1.097 ms	+0.4%
escrow/escrow-redeem_2-3	1.090 ms	1.095 ms	+0.5%
escrow/escrow-refund-1	447.8 μs	448.7 μs	+0.2%
future/future-increase-margin-1	581.7 μs	585.4 μs	+0.6%
future/future-increase-margin-2	809.0 μs	812.9 μs	+0.5%
future/future-increase-margin-3	1.375 ms	1.379 ms	+0.3%
future/future-increase-margin-4	1.376 ms	1.380 ms	+0.3%
future/future-increase-margin-5	1.419 ms	1.424 ms	+0.4%
future/future-increase-margin-6	1.866 ms	1.857 ms	-0.5%
future/future-pay-out-1	583.8 μs	581.3 μs	-0.4%
future/future-pay-out-2	815.6 μs	809.2 μs	-0.8%
future/future-pay-out-3	1.386 ms	1.385 ms	-0.1%
future/future-pay-out-4	1.382 ms	1.384 ms	+0.1%
future/future-pay-out-5	1.927 ms	1.935 ms	+0.4%
future/future-settle-early-1	584.8 μs	587.7 μs	+0.5%
future/future-settle-early-2	811.5 μs	814.4 μs	+0.4%
future/future-settle-early-3	1.383 ms	1.385 ms	+0.1%
future/future-settle-early-4	1.379 ms	1.385 ms	+0.4%
future/future-settle-early-5	1.538 ms	1.543 ms	+0.3%
game-sm/game-sm-success-1	1.013 ms	1.017 ms	+0.4%
game-sm/game-sm-success-2	605.8 μs	608.6 μs	+0.5%
game-sm/game-sm-success-3	1.460 ms	1.465 ms	+0.3%
game-sm/game-sm-success-4	722.7 μs	725.0 μs	+0.3%
game-sm/game-sm-success_2-1	1.012 ms	1.013 ms	+0.1%
game-sm/game-sm-success_2-2	608.6 μs	607.2 μs	-0.2%
game-sm/game-sm-success_2-3	1.474 ms	1.468 ms	-0.4%
game-sm/game-sm-success_2-4	727.7 μs	729.9 μs	+0.3%
game-sm/game-sm-success_2-5	1.470 ms	1.471 ms	+0.1%
game-sm/game-sm-success_2-6	728.4 μs	729.7 μs	+0.2%
multisig-sm/multisig-sm-01	1.062 ms	1.067 ms	+0.5%
multisig-sm/multisig-sm-02	1.066 ms	1.072 ms	+0.6%
multisig-sm/multisig-sm-03	1.082 ms	1.085 ms	+0.3%
multisig-sm/multisig-sm-04	1.095 ms	1.100 ms	+0.5%
multisig-sm/multisig-sm-05	1.287 ms	1.291 ms	+0.3%
multisig-sm/multisig-sm-06	1.061 ms	1.065 ms	+0.4%
multisig-sm/multisig-sm-07	1.062 ms	1.066 ms	+0.4%
multisig-sm/multisig-sm-08	1.078 ms	1.081 ms	+0.3%
multisig-sm/multisig-sm-09	1.093 ms	1.097 ms	+0.4%
multisig-sm/multisig-sm-10	1.280 ms	1.286 ms	+0.5%
ping-pong/ping-pong-1	800.5 μs	802.5 μs	+0.2%
ping-pong/ping-pong-2	801.3 μs	803.0 μs	+0.2%
ping-pong/ping-pong_2-1	495.6 μs	493.5 μs	-0.4%
prism/prism-1	476.5 μs	474.9 μs	-0.3%
prism/prism-2	1.089 ms	1.090 ms	+0.1%
prism/prism-3	996.3 μs	993.2 μs	-0.3%
pubkey/pubkey-1	415.8 μs	415.8 μs	0.0%
stablecoin/stablecoin_1-1	2.111 ms	2.118 ms	+0.3%
stablecoin/stablecoin_1-2	599.4 μs	600.9 μs	+0.3%
stablecoin/stablecoin_1-3	2.386 ms	2.386 ms	0.0%
stablecoin/stablecoin_1-4	650.7 μs	651.6 μs	+0.1%
stablecoin/stablecoin_1-5	3.010 ms	3.014 ms	+0.1%
stablecoin/stablecoin_1-6	833.6 μs	836.0 μs	+0.3%
stablecoin/stablecoin_2-1	2.101 ms	2.107 ms	+0.3%
stablecoin/stablecoin_2-2	598.9 μs	599.4 μs	+0.1%
stablecoin/stablecoin_2-3	2.376 ms	2.380 ms	+0.2%
stablecoin/stablecoin_2-4	647.8 μs	649.3 μs	+0.2%
token-account/token-account-1	555.0 μs	555.0 μs	0.0%
token-account/token-account-2	669.1 μs	668.9 μs	-0.0%
token-account/token-account-3	902.8 μs	904.0 μs	+0.1%
uniswap/uniswap-1	646.3 μs	647.2 μs	+0.1%
uniswap/uniswap-2	1.112 ms	1.115 ms	+0.3%
uniswap/uniswap-3	714.1 μs	715.1 μs	+0.1%
uniswap/uniswap-4	831.1 μs	832.1 μs	+0.1%
uniswap/uniswap-5	3.856 ms	3.860 ms	+0.1%
uniswap/uniswap-6	1.110 ms	1.110 ms	0.0%
uniswap/uniswap-7	2.800 ms	2.802 ms	+0.1%
uniswap/uniswap-8	1.059 ms	1.058 ms	-0.1%
vesting/vesting-1	899.8 μs	900.0 μs	+0.0%
marlowe/trustfund/trustfund-1	2.082 ms	2.086 ms	+0.2%
marlowe/trustfund/trustfund-2	1.531 ms	1.533 ms	+0.1%
marlowe/zerocoupon/zerocoupon-1	2.180 ms	2.184 ms	+0.2%
marlowe/zerocoupon/zerocoupon-2	1.326 ms	1.329 ms	+0.2%

kwxm · 2021-07-05T21:48:04Z

This looks like it maybe kind of works, but it seems to be doing an awful lot of git stuff and I'm not convinced that it's using the correct commit for the base. I'm sure that can be improved, but my ignorance of git is too profound for me to have any idea of what the right thing to do is. Also, it still keeps going back to a761044.

Anyway, the comparison script is doing the right thing, which is what this PR was supposed to be about. I should probably extract that and make a new PR that doesn't touch any of the CI stuff (later: done in #3494).

kwxm · 2021-07-08T10:56:27Z

/benchmark

iohk-devops · 2021-07-08T11:07:44Z

Comparing benchmark results of '38d9354bde98e27e9756471aae062e7a56ab857e' (base) and '66124c163084095f1dd6bd5720788cd055831946' (PR)

Script	`38d9354`	`66124c1`	Change
game-sm/game-sm-success-1	1.006 ms	1.000 ms	-0.6%
game-sm/game-sm-success-2	607.2 μs	604.1 μs	-0.5%
game-sm/game-sm-success-3	1.470 ms	1.465 ms	-0.3%
game-sm/game-sm-success-4	728.2 μs	727.2 μs	-0.1%
game-sm/game-sm-success_2-1	1.017 ms	1.019 ms	+0.2%
game-sm/game-sm-success_2-2	607.5 μs	609.0 μs	+0.2%
game-sm/game-sm-success_2-3	1.469 ms	1.475 ms	+0.4%
game-sm/game-sm-success_2-4	724.4 μs	725.9 μs	+0.2%
game-sm/game-sm-success_2-5	1.462 ms	1.466 ms	+0.3%
game-sm/game-sm-success_2-6	722.9 μs	725.9 μs	+0.4%

michaelpj · 2021-07-15T13:15:04Z

Do we still need this?

kwxm · 2021-07-15T15:20:32Z

Do we still need this?

I think the underlying issue hasn't been resolved yet, so let's keep it for the time being.

kwxm · 2021-08-01T10:41:02Z

I think the issues this was exploring have been fixed in #3611 and #3651, so I'm going to close it.

kwxm added 7 commits July 1, 2021 10:29

Adjust formatting

5d0fb8e

Update comment

069f2a3

Tiny change

2ee9ea9

Remove original version of bench-compare

90565c3

Update benchmark job

9d724ec

Switch back to PR branch before doing comparison

dcb82c2

Testing: what commits are we using?

3d0ec16

kwxm added 2 commits July 2, 2021 05:14

Wrong branch

abc49cd

Make branches more obvious

204a1cb

kwxm added the Do not merge label Jul 2, 2021

kwxm added 2 commits July 5, 2021 12:33

Update bench comparison script

f504489

More experiments with benchmark comparisons

c143264

Merge branch 'master' into kwxm/bench-compare-markdown

71850ec

Merge branch 'master' into kwxm/bench-compare-markdown

84fb35a

Let's see exactly what's going on

6eb2f00

Fix substring indices and run all benhcmarks

b8a4b08

kwxm mentioned this pull request Jul 6, 2021

Update bench comparison script #3494

Merged

11 tasks

michaelpj requested a review from gilligan July 6, 2021 10:33

Just look at game-sm to speed thigns up

66124c1

kwxm closed this Aug 1, 2021

kwxm deleted the kwxm/bench-compare-markdown branch August 1, 2021 10:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kwxm/bench compare markdown #3471

Kwxm/bench compare markdown #3471

kwxm commented Jul 2, 2021

kwxm commented Jul 2, 2021

kwxm commented Jul 2, 2021

kwxm commented Jul 2, 2021 •

edited

Loading

kwxm commented Jul 2, 2021 •

edited

Loading

michaelpj commented Jul 2, 2021

michaelpj commented Jul 2, 2021

kwxm commented Jul 2, 2021 •

edited

Loading

kwxm commented Jul 5, 2021

michaelpj commented Jul 5, 2021

kwxm commented Jul 5, 2021

kwxm commented Jul 5, 2021

kwxm commented Jul 5, 2021

kwxm commented Jul 5, 2021

iohk-devops commented Jul 5, 2021

kwxm commented Jul 5, 2021

iohk-devops commented Jul 5, 2021

kwxm commented Jul 5, 2021

iohk-devops commented Jul 5, 2021

kwxm commented Jul 5, 2021 •

edited

Loading

kwxm commented Jul 8, 2021

iohk-devops commented Jul 8, 2021

michaelpj commented Jul 15, 2021

kwxm commented Jul 15, 2021

kwxm commented Aug 1, 2021

Kwxm/bench compare markdown #3471

Kwxm/bench compare markdown #3471

Conversation

kwxm commented Jul 2, 2021

kwxm commented Jul 2, 2021

kwxm commented Jul 2, 2021

kwxm commented Jul 2, 2021 • edited Loading

kwxm commented Jul 2, 2021 • edited Loading

michaelpj commented Jul 2, 2021

michaelpj commented Jul 2, 2021

kwxm commented Jul 2, 2021 • edited Loading

kwxm commented Jul 5, 2021

michaelpj commented Jul 5, 2021

kwxm commented Jul 5, 2021

kwxm commented Jul 5, 2021

kwxm commented Jul 5, 2021

kwxm commented Jul 5, 2021

iohk-devops commented Jul 5, 2021

kwxm commented Jul 5, 2021

iohk-devops commented Jul 5, 2021

kwxm commented Jul 5, 2021

iohk-devops commented Jul 5, 2021

kwxm commented Jul 5, 2021 • edited Loading

kwxm commented Jul 8, 2021

iohk-devops commented Jul 8, 2021

michaelpj commented Jul 15, 2021

kwxm commented Jul 15, 2021

kwxm commented Aug 1, 2021

kwxm commented Jul 2, 2021 •

edited

Loading

kwxm commented Jul 2, 2021 •

edited

Loading

kwxm commented Jul 2, 2021 •

edited

Loading

kwxm commented Jul 5, 2021 •

edited

Loading