Skip to content

⚡ Bolt: single-pass chunk writing #3070

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

Closed
ChanTsune wants to merge 1 commit into from
Closed

⚡ Bolt: single-pass chunk writing #3070

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 69 additions & 6 deletions lib/src/chunk/write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,57 @@
//! Chunk writing and serialization to byte streams.

use crate::chunk::{Chunk, ChunkExt, ChunkType};
use crate::chunk::{ChunkType, Crc32, MIN_CHUNK_BYTES_SIZE};

#[cfg(feature = "unstable-async")]
use crate::chunk::Chunk;
use core::num::NonZeroU32;
#[cfg(feature = "unstable-async")]
use futures_io::AsyncWrite;
#[cfg(feature = "unstable-async")]
use futures_util::AsyncWriteExt;
use std::io::{self, Write};

pub(crate) struct CrcWriter<W> {
w: W,
crc: Crc32,
}

impl<W> CrcWriter<W> {
#[inline]
pub(crate) fn new(w: W) -> Self {
Self {
w,
crc: Crc32::new(),
}
}

#[inline]
pub(crate) fn finalize(self) -> u32 {
self.crc.finalize()
}
}

impl<W: Write> Write for CrcWriter<W> {
#[inline]
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let n = self.w.write(buf)?;
self.crc.update(&buf[..n]);
Ok(n)
}

#[inline]
fn flush(&mut self) -> io::Result<()> {
self.w.flush()
}

#[inline]
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
self.w.write_all(buf)?;
self.crc.update(buf);
Ok(())
}
}

pub(crate) struct ChunkWriter<W> {
w: W,
}
Expand All @@ -20,15 +64,29 @@
}

impl<W: Write> ChunkWriter<W> {
/// Writes a chunk in a single pass over the data.
///
/// This method is optimized for chunks where the CRC is not already known.
/// It calculates the CRC while writing the chunk type and data to the underlying writer.
#[inline]
pub(crate) fn write_chunk(&mut self, chunk: impl Chunk) -> io::Result<usize> {
chunk.write_chunk_in(&mut self.w)
pub(crate) fn write_chunk_single_pass(
&mut self,
ty: ChunkType,
data: &[u8],
) -> io::Result<usize> {
self.w.write_all(&(data.len() as u32).to_be_bytes())?;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The cast data.len() as u32 can truncate if data.len() exceeds u32::MAX. While the library defines MAX_CHUNK_DATA_LENGTH as u32::MAX, this function does not enforce it, which could lead to malformed chunks where the length field doesn't match the actual data written. Using u32::try_from provides a safer way to handle this. This approach also follows the preference for using specific io::ErrorKind for better semantic clarity.

Suggested change
self.w.write_all(&(data.len() as u32).to_be_bytes())?;
let len = u32::try_from(data.len()).map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "chunk data too large"))?;
self.w.write_all(&len.to_be_bytes())?;
References
  1. When reporting errors in Rust, using io::Error::new with a specific io::ErrorKind is preferred over io::Error::other with a generic message, as it provides better semantic clarity and aids in debugging and error handling.

let mut crc_writer = CrcWriter::new(&mut self.w);
crc_writer.write_all(ty.as_bytes())?;
crc_writer.write_all(data)?;
let crc = crc_writer.finalize();
self.w.write_all(&crc.to_be_bytes())?;
Ok(MIN_CHUNK_BYTES_SIZE + data.len())
}
}

#[cfg(feature = "unstable-async")]
impl<W: AsyncWrite + Unpin> ChunkWriter<W> {
pub(crate) async fn write_chunk_async(&mut self, chunk: impl Chunk) -> io::Result<usize> {

Check failure

Code scanning / clippy

no method named bytes_len found for type parameter impl Chunk in the current scope Error

no method named bytes_len found for type parameter impl Chunk in the current scope

Check failure

Code scanning / clippy

no method named bytes_len found for type parameter impl Chunk in the current scope Error

no method named bytes_len found for type parameter impl Chunk in the current scope
// write length
let length = chunk.length();
self.w.write_all(&length.to_be_bytes()).await?;
Expand All @@ -41,7 +99,7 @@

// write crc32
self.w.write_all(&chunk.crc().to_be_bytes()).await?;
Ok(chunk.bytes_len())

Check failure

Code scanning / clippy

no method named bytes_len found for type parameter impl Chunk in the current scope Error

no method named bytes_len found for type parameter impl Chunk in the current scope

Check failure

Code scanning / clippy

no method named bytes_len found for type parameter impl Chunk in the current scope Error

no method named bytes_len found for type parameter impl Chunk in the current scope
}
}

Expand Down Expand Up @@ -77,7 +135,7 @@
return Ok(0);
}
let chunk = &buf[..buf.len().min(self.max_chunk_size)];
self.w.write_chunk((self.ty, chunk))?;
self.w.write_chunk_single_pass(self.ty, chunk)?;
Ok(chunk.len())
}

Expand All @@ -96,7 +154,12 @@
#[test]
fn write_aend_chunk() {
let mut chunk_writer = ChunkWriter::new(Vec::new());
assert_eq!(chunk_writer.write_chunk((ChunkType::AEND, [])).unwrap(), 12);
assert_eq!(
chunk_writer
.write_chunk_single_pass(ChunkType::AEND, &[])
.unwrap(),
12
);
assert_eq!(
chunk_writer.w,
[0, 0, 0, 0, 65, 69, 78, 68, 107, 246, 72, 109]
Expand All @@ -108,7 +171,7 @@
let mut chunk_writer = ChunkWriter::new(Vec::new());
assert_eq!(
chunk_writer
.write_chunk((ChunkType::FDAT, b"text data"))
.write_chunk_single_pass(ChunkType::FDAT, b"text data")
.unwrap(),
21,
);
Expand Down
Loading