Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publishing benchmarks for graphs #740

Merged
merged 28 commits into from
Jul 31, 2023
Merged

Publishing benchmarks for graphs #740

merged 28 commits into from
Jul 31, 2023

Conversation

alvicsam
Copy link
Contributor

@alvicsam alvicsam commented Jul 24, 2023

PR adds gitlab and github jobs that run benchmarks for master and create nice graphs that can be found here

close https://github.com/paritytech/ci_cd/issues/848

@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Jul 24, 2023

BENCHMARKS

NATIVEWASMTIME
BENCHMARKMASTERPRDIFFMASTERPRDIFFWASMTIME OVERHEAD
execute/
bare_call_0
1.54ms 1.55ms ⚪ -0.36% 1.03ms 1.02ms ⚪ -0.51% 🟢 -34%
execute/
bare_call_0/typed
1.18ms 1.15ms ⚪ -0.78% 646.29µs 641.96µs ⚪ -0.67% 🟢 -44%
execute/
bare_call_1
1.68ms 1.60ms 🟢 -4.05% 1.20ms 1.17ms 🟢 -2.06% 🟢 -27%
execute/
bare_call_16
2.54ms 2.55ms ⚪ -0.21% 3.62ms 3.48ms 🟢 -3.82% 🟢 36%
execute/
bare_call_16/typed
1.62ms 1.61ms ⚪ -0.59% 1.81ms 1.76ms 🟢 -2.79% 🟢 10%
execute/
bare_call_1/typed
1.28ms 1.26ms ⚪ -2.18% 940.31µs 943.58µs ⚪ 0.38% 🟢 -25%
execute/
bare_call_4
1.84ms 1.78ms ⚪ -2.73% 1.56ms 1.56ms ⚪ 0.13% 🟢 -12%
execute/
bare_call_4/typed
1.32ms 1.26ms ⚪ -3.73% 921.68µs 921.92µs ⚪ 0.00% 🟢 -27%
execute/
br_table
1.37ms 1.33ms ⚪ -2.02% 1.20ms 1.13ms 🟢 -4.96% 🟢 -15%
execute/
count_until
620.92µs 622.33µs ⚪ 0.12% 1.37ms 1.37ms ⚪ 0.07% 🔴 120%
execute/
factorial_iterative
321.93µs 320.76µs ⚪ -0.30% 687.93µs 686.10µs ⚪ -0.21% 🔴 114%
execute/
factorial_recursive
513.91µs 512.93µs ⚪ -0.20% 852.13µs 892.66µs 🔴 4.59% 🟡 74%
execute/
fibonacci_iter
1.40ms 1.40ms ⚪ 0.13% 3.31ms 3.32ms ⚪ 0.16% 🔴 137%
execute/
fibonacci_rec
4.76ms 4.37ms 🟢 -8.23% 7.87ms 7.78ms 🟢 -1.14% 🟡 78%
execute/
fibonacci_tail
972.98µs 1.04ms 🔴 7.17% 1.89ms 1.89ms ⚪ -0.27% 🟡 81%
execute/
global_bump
716.85µs 724.08µs ⚪ 0.94% 1.67ms 1.67ms ⚪ 0.04% 🔴 130%
execute/
global_const
659.57µs 659.52µs ⚪ -0.03% 1.76ms 1.76ms ⚪ 0.04% 🔴 166%
execute/
host_calls
37.77µs 37.06µs ⚪ -2.57% 38.64µs 39.23µs 🔴 1.48% 🟢 6%
execute/
memory_fill
1.18ms 1.21ms ⚪ 1.30% 2.69ms 2.69ms ⚪ -0.11% 🔴 123%
execute/
memory_sum
1.14ms 1.23ms 🔴 5.21% 2.65ms 2.65ms ⚪ -0.06% 🔴 115%
execute/
memory_vec_add
2.41ms 2.30ms 🟢 -4.05% 5.71ms 5.72ms ⚪ 0.12% 🔴 149%
execute/
recursive_is_even
713.77µs 712.29µs ⚪ 0.22% 1.24ms 1.25ms ⚪ 0.48% 🟡 75%
execute/
recursive_ok
103.18µs 102.26µs ⚪ -0.96% 191.02µs 192.21µs ⚪ 0.65% 🟡 88%
execute/
recursive_scan
138.13µs 137.68µs ⚪ -0.37% 253.96µs 248.39µs ⚪ -1.97% 🟡 80%
execute/
recursive_trap
10.09µs 10.18µs ⚪ 0.86% 19.61µs 19.61µs ⚪ 0.13% 🟡 93%
execute/
regex_redux
455.22µs 459.94µs ⚪ 1.14% 998.79µs 970.46µs 🟢 -2.74% 🔴 111%
execute/
rev_complement
420.27µs 421.40µs ⚪ 0.30% 971.99µs 965.04µs ⚪ -0.75% 🔴 129%
execute/
tiny_keccak
317.14µs 315.10µs ⚪ -0.60% 859.12µs 862.28µs ⚪ 0.27% 🔴 174%
execute/
trunc_f2i
737.53µs 739.33µs ⚪ 0.23% 1.86ms 1.85ms ⚪ -0.26% 🔴 151%
instantiate/
wasm_kernel
54.18µs 53.08µs 🟢 -3.47% 56.32µs 59.74µs 🔴 6.60% 🟢 13%
translate/
erc1155
188.48µs 186.89µs ⚪ 0.05% 306.69µs 311.50µs 🔴 1.72% 🟡 67%
translate/
erc20
91.20µs 91.70µs ⚪ 0.41% 151.07µs 152.58µs ⚪ 0.93% 🟡 66%
translate/
erc721
128.98µs 128.75µs ⚪ -0.32% 215.97µs 218.26µs ⚪ 0.58% 🟡 70%
translate/
spidermonkey
56.13ms 56.16ms ⚪ 0.05% 0.00ns 0.00ns ⚪ 0.69% 🟢 -100%
translate/
wasm_kernel
3.76ms 3.75ms ⚪ -0.01% 5.73ms 5.76ms ⚪ 0.67% 🟡 54%

Link to pipeline

@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Merging #740 (91b7386) into master (4860ecc) will increase coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #740      +/-   ##
==========================================
+ Coverage   79.25%   79.40%   +0.15%     
==========================================
  Files         104      104              
  Lines        9046     9054       +8     
==========================================
+ Hits         7169     7189      +20     
+ Misses       1877     1865      -12     

see 16 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@alvicsam alvicsam changed the title [Do not merge] Publishing benchmarks for graphs Publishing benchmarks for graphs Jul 25, 2023
@alvicsam alvicsam requested a review from Robbepop July 25, 2023 14:41
@Robbepop
Copy link
Member

Robbepop commented Jul 26, 2023

@alvicsam thanks for implementing this so quickly and sorry for replying to late!

The graphs look pretty cool and the website display is clean.

I have a few extra requests if possible. 😇

  • Can I haz dark mode eventually (if not much work)? 😅
  • In most/all graphs the ns/iter column hosts a ton of zeros. Maybe if we use microseconds per iteration (us/iter) it would look a bit less overwhelming.
  • Maybe it a local problem with my screen but only at 50% zoom level I have a decent level of zoom on the graphs. At 100% I am basically looking at a single graph per page and I think this is simply too big given that we have so many of those graphs. We definitely want a zoom, but default zoom should provide a nice overview as in this picture:
image

With the zoom talk above it would be really cool with smaller zoom to have a high-level overview of all graphs and make it possible to click on them to enlarge a single graph for inspection if necessary. Is that possible?

@alvicsam
Copy link
Contributor Author

Can I haz dark mode eventually (if not much work)? 😅

Unfortunately I couldn't find this option in the GHA

In most/all graphs the ns/iter column hosts a ton of zeros. Maybe if we use microseconds per iteration (us/iter) it would look a bit less overwhelming.

With this action it's not possible, it's hardcoded there

Maybe it a local problem with my screen but only at 50% zoom level I have a decent level of zoom on the graphs.

No, this is how GHA generates the page with result. I've checked and have the same output: 2 columns with graphs on a big screen and 1 column on my laptop

@Robbepop
Copy link
Member

Can I haz dark mode eventually (if not much work)? 😅

Unfortunately I couldn't find this option in the GHA

Would it maybe be possible to write feature request issues for this? Not that important though.

In most/all graphs the ns/iter column hosts a ton of zeros. Maybe if we use microseconds per iteration (us/iter) it would look a bit less overwhelming.

With this action it's not possible, it's hardcoded there

Hmm, again I think we could write feature request issues since the ns/iter is really not a great default for everything anyways imo. Sometimes for example you measure throughput or measure network stuff which usually goes with ms/iter.

Copy link
Member

@Robbepop Robbepop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for implementing this so quickly @alvicsam! Sorry for taking unusually long to reply and review.

Comment on lines -11 to +12
.docker-env: &docker-env
image: "paritytech/ci-linux:staging"
.docker-env:
image: "paritytech/ci-unified:bullseye-1.71.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the effects of this and why was this changed/needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We replaced ci-linux with ci-unified. It has transparent tags naming, flexible choice of
Rust versions, and so on. More - https://hub.docker.com/repository/docker/paritytech/ci-unified

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are switching to our new ci-unified image. We decided to have "one for all" image and finally added versioning to it.

@Robbepop
Copy link
Member

@alvicsam Is this ready for merge?

@alvicsam
Copy link
Contributor Author

@alvicsam Is this ready for merge?

yes

@Robbepop Robbepop merged commit af8c588 into master Jul 31, 2023
17 checks passed
@Robbepop Robbepop deleted the as-bench branch July 31, 2023 10:23
@Robbepop
Copy link
Member

Thanks a lot @alvicsam for this! 🚀
Looking forward to seeing the benchmark graphs over time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants