From 61b12a4318a246022b592791b6a53d1e13b7361c Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 20 Jan 2023 04:46:47 +0100 Subject: [PATCH 1/3] Simplify chunk_encoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Firstly, use `[T]::chunks` rather than doing index calculation manually. Secondly, get rid of max_input_length. The function was overly complicated for no reason. After all, no matter whether engine uses padding or not, N-byte output buffer can accommodate (N/4*3) input bytes. Encode benchmarks here are kind of all over. Some improvements and some regressions: encode/ 3 29.272 ns -0.4% No change 50 54.162 ns +12.4% Regression 100 95.488 ns -7.8% Improvement 500 255.84 ns +3.5% Regression 3072 1.3527 µs +0.1% No change 3145728 1.4584 ms +0.2% Within noise 10485760 4.8923 ms +1.6% Regression 31457280 16.087 ms -2.6% Improvement encode_display/ 3 41.038 ns -6.9% Improvement 50 70.877 ns +7.2% Regression 100 91.190 ns +1.1% Within noise 500 264.58 ns -3.1% Improvement 3072 1.4515 µs -3.8% Improvement 3145728 1.4239 ms -0.2% Within noise 10485760 4.9768 ms -0.7% Within noise 31457280 18.202 ms -1.5% Improvement encode_reuse_buf/ 3 20.264 ns -23.1% Improvement 50 46.631 ns +2.1% Regression 100 66.405 ns -2.0% Improvement 500 243.52 ns +3.5% Regression 3072 1.3871 µs -1.2% Within noise 3145728 1.4278 ms +0.8% Within noise 10485760 5.0530 ms -1.6% Improvement 31457280 15.798 ms +0.7% No change encode_reuse_buf_stream/ 3 21.007 ns +1.1% Regression 50 56.542 ns -0.9% No change 100 75.214 ns -1.4% Improvement 500 247.61 ns -4.2% Improvement 3072 1.2836 µs +0.1% No change 3145728 1.3177 ms -0.5% Within noise 10485760 4.3843 ms +0.3% Within noise 31457280 14.058 ms -0.5% Within noise encode_slice/ 3 11.859 ns +0.1% No change 50 30.239 ns +2.8% Regression 100 53.383 ns -6.4% Improvement 500 207.90 ns +0.2% No change 3072 1.2050 µs -0.1% Within noise 3145728 1.2294 ms -0.5% Within noise 10485760 4.1025 ms -0.1% No change 31457280 12.352 ms +0.1% Within noise encode_string_reuse_buf_stream/ 3 39.237 ns -0.9% Within noise 50 100.15 ns -1.6% Improvement 100 124.03 ns +3.5% Regression 500 312.15 ns +10.0% Regression 3072 1.4781 µs +2.7% Regression 3145728 1.4942 ms +3.8% Regression 10485760 4.9965 ms +4.3% Regression 31457280 16.990 ms +7.4% Regression encode_string_stream/ 3 48.437 ns -4.2% Improvement 50 152.06 ns +3.5% Regression 100 170.02 ns -5.2% Improvement 500 324.46 ns -0.8% Within noise 3072 1.5332 µs +1.6% Regression 3145728 1.4947 ms +3.6% Regression 10485760 4.9710 ms +3.2% Regression 31457280 19.075 ms +2.2% Regression --- src/chunked_encoder.rs | 91 ++++++++---------------------------------- 1 file changed, 16 insertions(+), 75 deletions(-) diff --git a/src/chunked_encoder.rs b/src/chunked_encoder.rs index 0457259..c575a3a 100644 --- a/src/chunked_encoder.rs +++ b/src/chunked_encoder.rs @@ -1,10 +1,8 @@ #[cfg(any(feature = "alloc", feature = "std", test))] use alloc::string::String; -use core::cmp; #[cfg(any(feature = "alloc", feature = "std", test))] use core::str; -use crate::encode::add_padding; use crate::engine::{Config, Engine}; /// The output mechanism for ChunkedEncoder's encoded bytes. @@ -15,71 +13,39 @@ pub trait Sink { fn write_encoded_bytes(&mut self, encoded: &[u8]) -> Result<(), Self::Error>; } -const BUF_SIZE: usize = 1024; - /// A base64 encoder that emits encoded bytes in chunks without heap allocation. pub struct ChunkedEncoder<'e, E: Engine + ?Sized> { engine: &'e E, - max_input_chunk_len: usize, } impl<'e, E: Engine + ?Sized> ChunkedEncoder<'e, E> { pub fn new(engine: &'e E) -> ChunkedEncoder<'e, E> { - ChunkedEncoder { - engine, - max_input_chunk_len: max_input_length(BUF_SIZE, engine.config().encode_padding()), - } + ChunkedEncoder { engine } } pub fn encode(&self, bytes: &[u8], sink: &mut S) -> Result<(), S::Error> { - let mut encode_buf: [u8; BUF_SIZE] = [0; BUF_SIZE]; - let mut input_index = 0; - - while input_index < bytes.len() { - // either the full input chunk size, or it's the last iteration - let input_chunk_len = cmp::min(self.max_input_chunk_len, bytes.len() - input_index); - - let chunk = &bytes[input_index..(input_index + input_chunk_len)]; - - let mut b64_bytes_written = self.engine.internal_encode(chunk, &mut encode_buf); - - input_index += input_chunk_len; - let more_input_left = input_index < bytes.len(); - - if self.engine.config().encode_padding() && !more_input_left { - // no more input, add padding if needed. Buffer will have room because - // max_input_length leaves room for it. - b64_bytes_written += add_padding(bytes.len(), &mut encode_buf[b64_bytes_written..]); + const BUF_SIZE: usize = 1024; + const CHUNK_SIZE: usize = BUF_SIZE / 4 * 3; + + let mut buf = [0; BUF_SIZE]; + for chunk in bytes.chunks(CHUNK_SIZE) { + let mut len = self.engine.internal_encode(chunk, &mut buf); + if chunk.len() != CHUNK_SIZE && self.engine.config().encode_padding() { + // Final, potentially partial, chunk. Pad output to multiple of + // four bytes if required by config. + let padding = 4 - (len % 4); + if padding != 4 { + buf[len..(len + padding)].fill(crate::PAD_BYTE); + len += padding; + } } - - sink.write_encoded_bytes(&encode_buf[0..b64_bytes_written])?; + sink.write_encoded_bytes(&buf[..len])?; } Ok(()) } } -/// Calculate the longest input that can be encoded for the given output buffer size. -/// -/// If the config requires padding, two bytes of buffer space will be set aside so that the last -/// chunk of input can be encoded safely. -/// -/// The input length will always be a multiple of 3 so that no encoding state has to be carried over -/// between chunks. -fn max_input_length(encoded_buf_len: usize, padded: bool) -> usize { - let effective_buf_len = if padded { - // make room for padding - encoded_buf_len - .checked_sub(2) - .expect("Don't use a tiny buffer") - } else { - encoded_buf_len - }; - - // No padding, so just normal base64 expansion. - (effective_buf_len / 4) * 3 -} - // A really simple sink that just appends to a string #[cfg(any(feature = "alloc", feature = "std", test))] pub(crate) struct StringSink<'a> { @@ -151,31 +117,6 @@ pub mod tests { chunked_encode_matches_normal_encode_random(&helper); } - #[test] - fn max_input_length_no_pad() { - assert_eq!(768, max_input_length(1024, false)); - } - - #[test] - fn max_input_length_with_pad_decrements_one_triple() { - assert_eq!(765, max_input_length(1024, true)); - } - - #[test] - fn max_input_length_with_pad_one_byte_short() { - assert_eq!(765, max_input_length(1025, true)); - } - - #[test] - fn max_input_length_with_pad_fits_exactly() { - assert_eq!(768, max_input_length(1026, true)); - } - - #[test] - fn max_input_length_cant_use_extra_single_encoded_byte() { - assert_eq!(300, max_input_length(401, false)); - } - pub fn chunked_encode_matches_normal_encode_random(sink_test_helper: &S) { let mut input_buf: Vec = Vec::new(); let mut output_buf = String::new(); From d175648c0d5fb93d8f70a709303b5c671e575b4b Mon Sep 17 00:00:00 2001 From: Marshall Pierce <575695+marshallpierce@users.noreply.github.com> Date: Sun, 5 Feb 2023 10:11:57 -0700 Subject: [PATCH 2/3] Use add_padding now that it uses encoded len --- src/chunked_encoder.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/chunked_encoder.rs b/src/chunked_encoder.rs index c575a3a..6be02b5 100644 --- a/src/chunked_encoder.rs +++ b/src/chunked_encoder.rs @@ -1,10 +1,12 @@ +use crate::{ + encode::add_padding, + engine::{Config, Engine}, +}; #[cfg(any(feature = "alloc", feature = "std", test))] use alloc::string::String; #[cfg(any(feature = "alloc", feature = "std", test))] use core::str; -use crate::engine::{Config, Engine}; - /// The output mechanism for ChunkedEncoder's encoded bytes. pub trait Sink { type Error; @@ -31,13 +33,9 @@ impl<'e, E: Engine + ?Sized> ChunkedEncoder<'e, E> { for chunk in bytes.chunks(CHUNK_SIZE) { let mut len = self.engine.internal_encode(chunk, &mut buf); if chunk.len() != CHUNK_SIZE && self.engine.config().encode_padding() { - // Final, potentially partial, chunk. Pad output to multiple of - // four bytes if required by config. - let padding = 4 - (len % 4); - if padding != 4 { - buf[len..(len + padding)].fill(crate::PAD_BYTE); - len += padding; - } + // Final, potentially partial, chunk. + // Pad output to multiple of four bytes if required by config. + len += add_padding(len, &mut buf[len..]); } sink.write_encoded_bytes(&buf[..len])?; } From 96b29d848eae315e5540aa0325ad91937b65d620 Mon Sep 17 00:00:00 2001 From: Marshall Pierce <575695+marshallpierce@users.noreply.github.com> Date: Sat, 26 Aug 2023 06:19:06 -0600 Subject: [PATCH 3/3] Improve comments and increase test paranoia. --- src/chunked_encoder.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/chunked_encoder.rs b/src/chunked_encoder.rs index 6be02b5..69bc745 100644 --- a/src/chunked_encoder.rs +++ b/src/chunked_encoder.rs @@ -34,6 +34,8 @@ impl<'e, E: Engine + ?Sized> ChunkedEncoder<'e, E> { let mut len = self.engine.internal_encode(chunk, &mut buf); if chunk.len() != CHUNK_SIZE && self.engine.config().encode_padding() { // Final, potentially partial, chunk. + // Only need to consider if padding is needed on a partial chunk since full chunk + // is a multiple of 3, which therefore won't be padded. // Pad output to multiple of four bytes if required by config. len += add_padding(len, &mut buf[len..]); } @@ -121,7 +123,7 @@ pub mod tests { let mut rng = rand::rngs::SmallRng::from_entropy(); let input_len_range = Uniform::new(1, 10_000); - for _ in 0..5_000 { + for _ in 0..20_000 { input_buf.clear(); output_buf.clear();