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

Better plot.py with Python 3 & remove hard code #569

Merged
merged 4 commits into from
Oct 1, 2020

Conversation

JakkuSakura
Copy link
Contributor

I was trying to add my own crate into the benchmark and found out that this script was hard to read and modify. So I converted it into python 3 and removed most of the hardcode.

ax.barh([y+1 for y in ys], mpsc, color='black', **opts)
ax.barh([y+2 for y in ys], futures_channel, color='blue', **opts)
for (i, score) in enumerate(scores.values()):
ax.barh([y + i - len(scores) // 2 for y in ys], score, **opts)
Copy link
Member

Choose a reason for hiding this comment

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

We're losing the different colors here, perhaps have a fixed colors list that you index with i?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Matplotlib will allocate different colors automatically

@Vtec234
Copy link
Member

Vtec234 commented Sep 20, 2020

Looks good, thanks! Could you please post an example plot created with the new version just to confirm it's comparable to the existing ones?

@JakkuSakura
Copy link
Contributor Author

Currently, it needs some tweaks to display better. But it's fine I think
image

@Vtec234
Copy link
Member

Vtec234 commented Sep 22, 2020

It seems there are a few problems:

  • non-existent benchmarks are default initialized to 0 and then shown as empty bars, but they shouldn't be displayed at all
  • the X-axis ticks are gone (just 0 on the left)
  • some of the horizontal bars are too long, and as you can see the text overlaps with other graphs

With those fixed, I think it'd be good to go.

@JakkuSakura
Copy link
Contributor Author

JakkuSakura commented Sep 23, 2020 via email

@JakkuSakura
Copy link
Contributor Author

Now the new version has come out
image

Copy link
Member

@Vtec234 Vtec234 left a comment

Choose a reason for hiding this comment

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

LGTM. I made a small change to have cleaner names like mpsc instead of Rust_mpsc, and chan (Go) instead of Go_chan. I will merge in a week or so if noone objects.

@Vtec234
Copy link
Member

Vtec234 commented Oct 1, 2020

bors r+

bors bot added a commit that referenced this pull request Oct 1, 2020
569: Better plot.py with Python 3 & remove hard code r=Vtec234 a=qiujiangkun

I was trying to add my own crate into the benchmark and found out that this script was hard to read and modify. So I converted it into python 3 and removed most of the hardcode.

Co-authored-by: QiuJiangkun <qiujiangkun@foxmail.com>
Co-authored-by: Wojciech Nawrocki <wjnawrocki+gh@protonmail.com>
@bors
Copy link
Contributor

bors bot commented Oct 1, 2020

Build failed:

@Vtec234
Copy link
Member

Vtec234 commented Oct 1, 2020

502 on crates.io, merging manually.

@Vtec234 Vtec234 merged commit 7cc8377 into crossbeam-rs:master Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants