Skip to content

Add grapheme iteration benchmarks for various languages.#78

Merged
Manishearth merged 3 commits into
unicode-rs:masterfrom
cessen:grapheme_bench
Feb 14, 2020
Merged

Add grapheme iteration benchmarks for various languages.#78
Manishearth merged 3 commits into
unicode-rs:masterfrom
cessen:grapheme_bench

Conversation

@cessen

@cessen cessen commented Feb 14, 2020

Copy link
Copy Markdown
Contributor

Grapheme iteration benchmarks for Arabic, English, Hindi, Japanese, Korean, Mandarin Chinese, Russian, and C source code.

@cessen

cessen commented Feb 14, 2020

Copy link
Copy Markdown
Contributor Author

Here are the cleaned up benchmarks. They aren't quite the same as the ones I used in #77:

  • I basically re-built them to make extra sure all the text is from open-source/free-culture sources.
  • I took the time to source enough text to make most of the text unique, rather than a bunch of repeat copy-paste.
  • I reduced the text size to ~50kb each, as that seems like plenty.
  • I removed the zalgo and worst-case texts, as I don't think they're actually useful in practice and would likely just be confusing to people looking at the benchmarks in the future.

I wasn't totally sure how best to exclude the benches folder from publishing. Simply adding it to the exclude list actually causes packaging to fail, since its referenced by the [[bench]] section. So for now I only excluded benches/texts, which contains the large text files. If I should do this differently, let me know!

@cessen

cessen commented Feb 14, 2020

Copy link
Copy Markdown
Contributor Author

Also, just to be clear: the benchmarks don't compile without the text files present. So I'm a bit nervous about this approach for excluding the benchmark texts from publishing. I'm not sure if there's any infrastructure that might not like that.

@Manishearth Manishearth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great! Can we mention the license for these files? It's CC-BY-SA so we need to mention the license and attribute it.

Comment thread benches/graphemes.rs Outdated

fn graphemes_english(bench: &mut Bencher) {
bench.iter(|| {
for g in UnicodeSegmentation::graphemes(TEXT_ENGLISH, true) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the text itself should also pass through black_box. Probably doesn't matter given how large it is, but worth a shot.

Alternatively, we can load the file dynamically outside of the iter() call.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pushed a fix that does this.

@Manishearth

Copy link
Copy Markdown
Member

We've had complaints about tests that break when run from published code, but this is usually packagers ensuring that their packaging went correctly. I don't think we'll have the same issue for benches. However, my understanding is that you don't need a [[bench]] entry if you just have a benches/ folder anyway, so we could totally just exclude it.

@Manishearth

Copy link
Copy Markdown
Member

Oh, nope, it's necessary because you're using a custom harness. Darn.

@Manishearth

Copy link
Copy Markdown
Member

Anyway, the benches will now compile, just not run if run from the package.

@Manishearth Manishearth merged commit 485767a into unicode-rs:master Feb 14, 2020
@Manishearth

Copy link
Copy Markdown
Member

Pushed in some commits adding a license/attribution and making the benchmarks use files. Thanks for doing this!

@cessen

cessen commented Feb 15, 2020

Copy link
Copy Markdown
Contributor Author

Awesome, thanks for cleaning this up!

Out of curiosity, is there a way to do benchmarks in rust without a custom harness? My understanding was that the standard benchmarker isn't stable, so you always need a custom harness. If that's not the case, I'd be more than happy to change the benchmarker that's used.

@cessen cessen deleted the grapheme_bench branch February 15, 2020 00:49
@Manishearth

Copy link
Copy Markdown
Member

I didn't know that you can use custom harnesses this way on stable!

Yes, the default harness isn't stable, but most people just set up their CI to bench on nightly only. shrug

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.

2 participants