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.

Sign up for GitHub

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

Jump to bottom

Minimize unsafe usage and rework line wrapping #31

Merged
merged 12 commits into from
May 22, 2017
Merged

Minimize unsafe usage and rework line wrapping #31

merged 12 commits into from
May 22, 2017

Conversation

marshallpierce
Copy link
Owner

@marshallpierce marshallpierce commented May 14, 2017 •
edited
Loading

Builds on #27 to minimize unsafe usage (see discussion in #29 for more context).

This has only a minor performance cost, and on certain sizes, a small boost:

 name                    master.bcmp ns/iter    branch.bcmp ns/iter     diff ns/iter  diff % 
 encode_100b             94 (1063 MB/s)          87 (1149 MB/s)                    -7  -7.45% 
 encode_100b_reuse_buf   72 (1388 MB/s)          67 (1492 MB/s)                    -5  -6.94% 
 encode_10mib            8,894,048 (1178 MB/s)   9,030,736 (1161 MB/s)        136,688   1.54% 
 encode_10mib_reuse_buf  5,685,071 (1844 MB/s)   5,991,851 (1750 MB/s)        306,780   5.40% 
 encode_30mib            27,063,216 (1162 MB/s)  27,540,693 (1142 MB/s)       477,477   1.76% 
 encode_30mib_reuse_buf  17,055,237 (1844 MB/s)  18,557,631 (1695 MB/s)     1,502,394   8.81% 
 encode_3b               34 (88 MB/s)            38 (78 MB/s)                       4  11.76% 
 encode_3b_reuse_buf     14 (214 MB/s)           17 (176 MB/s)                      3  21.43% 
 encode_3kib             1,711 (1795 MB/s)       1,634 (1880 MB/s)                -77  -4.50% 
 encode_3kib_reuse_buf   1,691 (1816 MB/s)       1,611 (1906 MB/s)                -80  -4.73% 
 encode_3mib             2,436,637 (1291 MB/s)   2,518,086 (1249 MB/s)         81,449   3.34% 
 encode_3mib_reuse_buf   1,692,495 (1858 MB/s)   1,704,400 (1845 MB/s)         11,905   0.70% 
 encode_500b             313 (1597 MB/s)         294 (1700 MB/s)                  -19  -6.07% 
 encode_500b_reuse_buf   292 (1712 MB/s)         274 (1824 MB/s)                  -18  -6.16% 
 encode_50b              62 (806 MB/s)           61 (819 MB/s)                     -1  -1.61% 
 encode_50b_reuse_buf    42 (1190 MB/s)          41 (1219 MB/s)                    -1  -2.38%

Also, mime (which line wraps) is now only somewhat slower instead of much slower:

test encode_3kib_reuse_buf      ... bench:       1,612 ns/iter (+/- 12) = 1905 MB/s
test encode_3kib_reuse_buf_mime ... bench:       2,017 ns/iter (+/- 4) = 1523 MB/s
test encode_500b_reuse_buf      ... bench:         278 ns/iter (+/- 1) = 1798 MB/s
test encode_500b_reuse_buf_mime ... bench:         378 ns/iter (+/- 2) = 1322 MB/s

marshallpierce and others added 10 commits April 30, 2017 14:15
@marshallpierce
Add benchmarks for line wrapping
0c15adf
@marshallpierce
Rewrite line wrapping to use a linear-time algo.
f775196 
Encoded bytes are moved from the end to the front so each byte is
only moved once.

Encoding is somewhat rearranged to operate on a slice into the
output buffer. This makes it easier to avoid clobbering any
existing bytes in the buffer, as well as paving the way to slice-
based encoding needed for a Display wrapper, stream adapters, etc.
@marshallpierce
Merge branch 'master' into line-wrap
adfde3c
@marshallpierce
A few more expect()'s in encode_config_buf
a72d575
@marshallpierce
Add explanatory comment
1683292
Minimize unsafe use on the basic encode code path.
cab488a 
At the cost of a 30% encode speed hit, the only unsafe used outside
of line wrapping is to view the output buffer as a Vec<u8>.
Help the optimizer by making some intermediate slices.
eeb9f5c 
Hand unroll the main encode loop 4x. 8x was barely better on longer
inputs, but only barely, and it hurt shorter inputs that then couldn't
enter the loop at all.
Use safemem's ptr::copy wrapper in line_wrap.
184bf14
Use a const for number of blocks per encode fast loop
b698641
Tests ensuring that encoding and padding produce utf8.
bb31229
@marshallpierce marshallpierce mentioned this pull request May 14, 2017
Closed
@marshallpierce
Syntactic tidying, plus removing some tests.
91366d5 
The old tests that exhaustively check strings a couple bytes long
weren't that useful, and only checked one config. Using the random
config helper in src/tests.rs is a better use of wall clock time
when waiting for tests to run.
@marshallpierce
A couple more tiny syntax changes.
8ed5dde
@marshallpierce
Copy link
Owner Author

@alicemaz I think this is good to go; would you like more time to review or should I merge?

@alicemaz
Copy link
Collaborator

looks good to me

@alicemaz alicemaz merged commit 1e23ea9 into marshallpierce:master May 22, 2017
@alicemaz alicemaz mentioned this pull request May 22, 2017
Closed
@marshallpierce marshallpierce deleted the minimize-unsafe branch May 22, 2017 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marshallpierce @alicemaz