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

Design Discussion: Make each Config a distinct type #89

Closed
ggriffiniii opened this issue Dec 6, 2018 · 2 comments
Closed

Design Discussion: Make each Config a distinct type #89

ggriffiniii opened this issue Dec 6, 2018 · 2 comments
Assignees

Comments

@ggriffiniii
Copy link
Contributor

Currently the Config is a struct that contains a CharacterSet and a boolean for padding. CharacterSet is an enum containing the 3 supported alphabets.

I've been toying around with the idea of changing the layout of the Config. The general idea is to make each supported alphabet a distinct type. Each type would then implement some traits to support the encoding/decoding algorithms. I believe with proper inlining this would allow the compiler to eliminate some branches and optimize the code better. I've sketched out a rough prototype of this and the initial results seem to align with expectations.

Encoding benchmarks:

bench_small_input/encode_slice/3                                                                             
                        time:   [8.1793 ns 8.2160 ns 8.2604 ns]
                        thrpt:  [346.36 MiB/s 348.23 MiB/s 349.79 MiB/s]
                 change:
                        time:   [-30.505% -29.744% -28.870%] (p = 0.00 < 0.05)
                        thrpt:  [+40.588% +42.337% +43.895%]
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe
bench_small_input/encode_slice/50                                                                            
                        time:   [37.009 ns 37.237 ns 37.508 ns]
                        thrpt:  [1.2415 GiB/s 1.2505 GiB/s 1.2582 GiB/s]
                 change:
                        time:   [-13.650% -12.534% -11.558%] (p = 0.00 < 0.05)
                        thrpt:  [+13.068% +14.330% +15.808%]
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  3 (3.00%) low mild
  4 (4.00%) high mild
  6 (6.00%) high severe
bench_small_input/encode_slice/100                                                                            
                        time:   [60.019 ns 60.255 ns 60.534 ns]
                        thrpt:  [1.5385 GiB/s 1.5456 GiB/s 1.5517 GiB/s]
                 change:
                        time:   [-7.1787% -6.7605% -6.3371%] (p = 0.00 < 0.05)
                        thrpt:  [+6.7658% +7.2507% +7.7339%]
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe
bench_small_input/encode_slice/500                                                                            
                        time:   [280.64 ns 281.54 ns 282.68 ns]
                        thrpt:  [1.6473 GiB/s 1.6540 GiB/s 1.6593 GiB/s]
                 change:
                        time:   [-5.6106% -5.0493% -4.4584%] (p = 0.00 < 0.05)
                        thrpt:  [+4.6665% +5.3179% +5.9440%]
                        Performance has improved.

decoding and larger sizes also improve but by much more marginal amounts ~2%. I'm happy to polish it and submit it as a pull request if you're interested, but seeing as it's a pretty substantial change I figured I would open an issue for discussion first.

@marshallpierce
Copy link
Owner

Interesting; also seems like potentially a good way to address #87 too, like you said. My time is pretty limited so you might want to wait until I've gotten to #88 so you don't have work waiting for a long time, as I know that's frustrating, but I do think this idea is worth exploring. Just remember to leave design room for potentially having a trait for scalar vs various SIMD flavors for the underlying byte shuffling.

@ggriffiniii
Copy link
Contributor Author

Again. I'm going to close this just as a cleanup. These ideas have been implemented and exist in the radix64 crate https://githhub.com/ggriffiniii/radix64 if you're interested

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

No branches or pull requests

2 participants