Fix a bug in reorder_line() and improve benchmarks

behnam commented 7 years ago

Summary of changes:

Fix bug in reorder_line() where optimized branch returned the full text (instead of line text), after (supposedly) reorder.
Improve benchmarks logic to measure reorder_line() in isolation, and for full text of the test.
Add new basic benchmarks to be able to measure perf for small, common text strings.
Move serde_test to dev-dependencies, as it's only used in test profile.
Bump version to 0.3.2.

This change is

behnam commented 7 years ago

New benchmark results:

Travis CI

     Running `/home/travis/build/behnam/rust-unicode-bidi/target/release/deps/basic-15095c098774ae71 --bench`
running 4 tests
test bench_1_bidi_info_new_for_ltr_texts  ... bench:       1,914 ns/iter (+/- 32)
test bench_2_bidi_info_new_for_bidi_texts ... bench:       2,495 ns/iter (+/- 212)
test bench_3_reorder_line_for_ltr_texts   ... bench:         263 ns/iter (+/- 6)
test bench_4_reorder_line_for_bidi_texts  ... bench:         622 ns/iter (+/- 8)
test result: ok. 0 passed; 0 failed; 0 ignored; 4 measured; 0 filtered out
     Running `/home/travis/build/behnam/rust-unicode-bidi/target/release/deps/udhr-9ad6078eca3a68ee --bench`
running 4 tests
test bench_1_bidi_info_new_for_ltr_texts  ... bench:     834,234 ns/iter (+/- 53,219)
test bench_2_bidi_info_new_for_bidi_texts ... bench:   1,069,752 ns/iter (+/- 24,763)
test bench_3_reorder_line_for_ltr_texts   ... bench:     176,447 ns/iter (+/- 5,069)
test bench_4_reorder_line_for_bidi_texts  ... bench:     464,879 ns/iter (+/- 14,333)

Local Machine

     Running target/release/deps/basic-e727a8b30b644dba

running 4 tests
test bench_1_bidi_info_new_for_ltr_texts  ... bench:       1,689 ns/iter (+/- 183)
test bench_2_bidi_info_new_for_bidi_texts ... bench:       2,124 ns/iter (+/- 256)
test bench_3_reorder_line_for_ltr_texts   ... bench:         233 ns/iter (+/- 25)
test bench_4_reorder_line_for_bidi_texts  ... bench:         573 ns/iter (+/- 107)

test result: ok. 0 passed; 0 failed; 0 ignored; 4 measured

     Running target/release/deps/udhr-c7491ea72b980ef8

running 4 tests
test bench_1_bidi_info_new_for_ltr_texts  ... bench:     714,457 ns/iter (+/- 236,974)
test bench_2_bidi_info_new_for_bidi_texts ... bench:     861,271 ns/iter (+/- 108,180)
test bench_3_reorder_line_for_ltr_texts   ... bench:     108,812 ns/iter (+/- 20,020)
test bench_4_reorder_line_for_bidi_texts  ... bench:     370,170 ns/iter (+/- 48,696)

mbrubeck commented 7 years ago

@bors-servo r+

bors-servo commented 7 years ago

:pushpin: Commit c8da6f6 has been approved by mbrubeck

bors-servo commented 7 years ago

:hourglass: Testing commit c8da6f62e1e690e90b38dc0790a9f977a83b7785 with merge af56fb0b2ee3cd5e90d1b7dff5fc0beffbc095af...

behnam commented 7 years ago

@bors-servo r+

bors-servo commented 7 years ago

@behnam: :key: Insufficient privileges: Not in reviewers

behnam commented 7 years ago

🔑 Insufficient privileges: Not in reviewers

That's a bummer!

@mbrubeck, looks like I have to bother you for another r+ here. I added a couple of more tests to one of the middle commits and you stamped last.

jdm commented 7 years ago

@bors: r=mbrubeck

KiChjang commented 7 years ago

@bors-servo r=mbrubeck

bors-servo commented 7 years ago

:pushpin: Commit 6b20563 has been approved by mbrubeck

bors-servo commented 7 years ago

:hourglass: Testing commit 6b20563ae4b6a521acf3c26cb47d49bdc6e1b559 with merge 36c0603d9609713c44cfb32d9c360b1efff8852a...

bors-servo commented 7 years ago

:sunny: Test successful - status-travis Approved by: mbrubeck Pushing 36c0603d9609713c44cfb32d9c360b1efff8852a to master...

behnam commented 7 years ago

Tagged and released on merge commit:

servo / unicode-bidi

Fix a bug in reorder_line() and improve benchmarks #35

Travis CI

Local Machine