Skip to content

chacha20: Replace salsa20-core with ctr-derived buffering#81

Merged
tarcieri merged 1 commit into
masterfrom
chacha20/remove-salsa20-core
Jan 15, 2020
Merged

chacha20: Replace salsa20-core with ctr-derived buffering#81
tarcieri merged 1 commit into
masterfrom
chacha20/remove-salsa20-core

Conversation

@tarcieri

@tarcieri tarcieri commented Jan 15, 2020

Copy link
Copy Markdown
Member

The previously used buffering logic in the salsa20-core crate is complex, hard-to-audit, and slow:

https://github.com/RustCrypto/stream-ciphers/blob/6be62af/salsa20-core/src/lib.rs#L93

The main reason is because it tries to handle too many things at once, namely:

  • serializing the ChaCha20 block function output as little endian bytes
  • implementing the stream-cipher APIs

This commit gets rid of salsa20-core as a dependency (which is also making it difficult to do further SIMD optimizations), replacing it with code derived from the ctr crate, but specialized to the ChaCha20 use case. It also changes the Block::generate API to write into a referenced slice as output, handling little endian serialization (which for the SSE2 backend is a noop), allowing the buffering code to work in terms of bytes rather than u32 arrays.

This results in a ~40% performance improvement (bringing us from ~3.5cpb to ~2.4cpb):

chacha20/apply_keystream/1024
                        time:   [2504.7967 cycles 2524.0030 cycles 2547.6807 cycles]
                        thrpt:  [2.4880 cpb 2.4648 cpb 2.4461 cpb]
                 change:
                        time:   [-32.266% -30.047% -27.734%] (p = 0.00 < 0.05)
                        thrpt:  [+38.378% +42.952% +47.637%]
                        Performance has improved.

chacha20/apply_keystream/2048
                        time:   [4980.9154 cycles 5002.3081 cycles 5025.7369 cycles]
                        thrpt:  [2.4540 cpb 2.4425 cpb 2.4321 cpb]
                 change:
                        time:   [-38.110% -36.161% -33.880%] (p = 0.00 < 0.05)
                        thrpt:  [+51.239% +56.644% +61.576%]
                        Performance has improved.

chacha20/apply_keystream/4096
                        time:   [9873.7431 cycles 9912.0935 cycles 9957.6095 cycles]
                        thrpt:  [2.4311 cpb 2.4199 cpb 2.4106 cpb]
                 change:
                        time:   [-30.936% -29.335% -27.274%] (p = 0.00 < 0.05)
                        thrpt:  [+37.502% +41.513% +44.792%]
                        Performance has improved.

chacha20/apply_keystream/8192
                        time:   [19833.5433 cycles 19911.5519 cycles 20008.2953 cycles]
                        thrpt:  [2.4424 cpb 2.4306 cpb 2.4211 cpb]
                 change:
                        time:   [-30.941% -29.657% -27.959%] (p = 0.00 < 0.05)
                        thrpt:  [+38.809% +42.160% +44.803%]
                        Performance has improved.

chacha20/apply_keystream/16384
                        time:   [39444.3470 cycles 39654.3508 cycles 39920.3374 cycles]
                        thrpt:  [2.4365 cpb 2.4203 cpb 2.4075 cpb]
                 change:
                        time:   [-30.878% -29.276% -27.000%] (p = 0.00 < 0.05)
                        thrpt:  [+36.986% +41.395% +44.672%]
                        Performance has improved.

The previously used buffering logic in the `salsa20-core` crate is
complex, hard-to-audit, and slow:

https://github.com/RustCrypto/stream-ciphers/blob/6be62af/salsa20-core/src/lib.rs#L93

The main reason is because it tries to handle too many things at once,
namely:

- serializing the ChaCha20 block function output as little endian bytes
- implementing the `stream-cipher` APIs

This commit gets rid of `salsa20-core` as a dependency (which is also
making it difficult to do further SIMD optimizations), replacing it with
code derived from the `ctr` crate, but specialized to the ChaCha20 use
case.
@tarcieri
tarcieri force-pushed the chacha20/remove-salsa20-core branch from 1e1f36c to c0740fe Compare January 15, 2020 23:38
@tarcieri
tarcieri merged commit 98217d5 into master Jan 15, 2020
@tarcieri
tarcieri deleted the chacha20/remove-salsa20-core branch January 15, 2020 23:42
@tarcieri tarcieri mentioned this pull request Jan 16, 2020
tarcieri added a commit that referenced this pull request Jan 17, 2020
This is pretty much the same PR as #81 was for `chacha20`.

The previously used buffering logic in the `salsa20-core` crate is
complex, hard-to-audit, and slow:

https://github.com/RustCrypto/stream-ciphers/blob/6be62af/salsa20-core/src/lib.rs#L93

The main reason is because it tries to handle too many things at once,
namely:

- serializing the Salsa20 block function output as little endian bytes
- implementing the stream-cipher APIs

This commit gets rid of `salsa20-core` as a dependency, replacing it
with code derived from the `ctr` crate, but specialized to the Salsa20
use case.

Ideally this code could eventually be unified with the extremely similar
code in the `ctr` crate and `chacha20` crates, as much of it seems
reusable.

For now though, I think it's probably better to keep the code in
`chacha20` separate from this code: the previous API was a major
impediment in SIMD optimizations, and being able to refactor both the
buffering logic and the various block function implementations
internally to a single crate is much easier than having to touch
(and release) 3 different crates for such a change.

This crate doesn't have a SIMD backend, and there's no plans to add one,
so using the simplest buffering logic possible probably makes sense.

As this was the last crate with a code dependency on `salsa20-core`,
this commit also removes it, as it's no longer needed.
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.

1 participant