Skip to content

Commit

Permalink
Rust: make mcap-rs wasm compatible (#989)
Browse files Browse the repository at this point in the history
### Public-Facing Changes

* Dependency is changed from
[lz4](https://github.com/10xGenomics/lz4-rs) to
[lz4_flex](https://github.com/PSeitz/lz4_flex)
* A new feature flag `lz4` is introduced and enabled by default

### Description
This change makes mcap-rs wasm compatible.

* Replace [lz4](https://github.com/10xGenomics/lz4-rs) with
[lz4_flex](https://github.com/PSeitz/lz4_flex), because the former one
depends on `lz4-sys`, which is not easy to port to wasm.
* Add a feature flag `lz4` so the library user can choose to opt-out of
the dependency.
* ~~Turn off the default features and~~ turn on the `wasm` feature for
[zstd](https://github.com/gyscos/zstd-rs) when the target platform is
wasm.
* Add Clippy checking in the CI for all combinations of features.
* Minor fixes for Clippy linter warnings.
  • Loading branch information
JiangengDong authored Oct 31, 2023
1 parent bb31acd commit 6fd068f
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ jobs:
components: "rustfmt, clippy"
- run: cargo fmt --all -- --check
- run: cargo clippy -- --no-deps
- run: cargo clippy --no-default-features -- --no-deps
- run: cargo clippy --no-default-features --features lz4 -- --no-deps
- run: cargo clippy --no-default-features --features zstd -- --no-deps
- run: cargo build
- run: cargo test
- name: "publish to crates.io"
Expand Down
10 changes: 8 additions & 2 deletions rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,21 @@ byteorder = "1.4"
crc32fast = "1.3"
enumset = "1.0.11"
log = "0.4"
lz4 = "1.0"
num_cpus = "1.13"
paste = "1.0"
thiserror = "1.0"
lz4_flex = { version = "0.11.1", optional = true }

[target.'cfg(target_arch = "wasm32")'.dependencies]
zstd = { version = "0.11", features = ["wasm"], optional = true }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
zstd = { version = "0.11", features = ["zstdmt"], optional = true }

[features]
default = ["zstd"]
default = ["zstd", "lz4"]
zstd = ["dep:zstd"]
lz4 = ["dep:lz4_flex"]

[dev-dependencies]
anyhow = "1"
Expand Down
4 changes: 4 additions & 0 deletions rust/src/io_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ pub struct CountingCrcReader<R> {
}

impl<R: Read> CountingCrcReader<R> {
/// Creates a new `CountingCrcReader` with the given reader.
///
/// This is not used when both `lz4` and `zstd` features are disabled.
#[allow(dead_code)]
pub fn new(inner: R) -> Self {
Self {
inner,
Expand Down
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ pub const MAGIC: &[u8] = &[0x89, b'M', b'C', b'A', b'P', 0x30, b'\r', b'\n'];
pub enum Compression {
#[cfg(feature = "zstd")]
Zstd,
#[cfg(feature = "lz4")]
Lz4,
}

Expand Down
8 changes: 7 additions & 1 deletion rust/src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ fn read_record(op: u8, body: &[u8]) -> McapResult<records::Record<'_>> {

enum ChunkDecompressor<'a> {
Null(LinearReader<'a>),
/// This is not used when both `zstd` and `lz4` features are disabled.
#[allow(dead_code)]
Compressed(Option<CountingCrcReader<Box<dyn Read + Send + 'a>>>),
}

Expand All @@ -274,10 +276,14 @@ impl<'a> ChunkReader<'a> {
#[cfg(not(feature = "zstd"))]
"zstd" => panic!("Unsupported compression format: zstd"),

#[cfg(feature = "lz4")]
"lz4" => ChunkDecompressor::Compressed(Some(CountingCrcReader::new(Box::new(
lz4::Decoder::new(data)?,
lz4_flex::frame::FrameDecoder::new(data),
)))),

#[cfg(not(feature = "lz4"))]
"lz4" => panic!("Unsupported compression format: lz4"),

"" => {
if header.uncompressed_size != header.compressed_size {
warn!(
Expand Down
1 change: 1 addition & 0 deletions rust/src/records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ fn parse_vec<T: BinRead<Args<'static> = ()>>() -> BinResult<Vec<T>> {
Ok(parsed)
}

#[allow(clippy::ptr_arg)]
#[binrw::writer(writer, endian)]
fn write_vec<T: BinWrite<Args<'static> = ()>>(v: &Vec<T>) -> BinResult<()> {
use std::io::SeekFrom;
Expand Down
25 changes: 15 additions & 10 deletions rust/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,8 @@ enum Compressor<W: Write> {
Null(W),
#[cfg(feature = "zstd")]
Zstd(zstd::Encoder<'static, W>),
Lz4(lz4::Encoder<W>),
#[cfg(feature = "lz4")]
Lz4(lz4_flex::frame::FrameEncoder<W>),
}

impl<W: Write> Compressor<W> {
Expand All @@ -658,11 +659,8 @@ impl<W: Write> Compressor<W> {
Compressor::Null(w) => w,
#[cfg(feature = "zstd")]
Compressor::Zstd(w) => w.finish()?,
Compressor::Lz4(w) => {
let (w, err) = w.finish();
err?;
w
}
#[cfg(feature = "lz4")]
Compressor::Lz4(w) => w.finish()?,
})
}
}
Expand All @@ -673,6 +671,7 @@ impl<W: Write> Write for Compressor<W> {
Compressor::Null(w) => w.write(buf),
#[cfg(feature = "zstd")]
Compressor::Zstd(w) => w.write(buf),
#[cfg(feature = "lz4")]
Compressor::Lz4(w) => w.write(buf),
}
}
Expand All @@ -682,6 +681,7 @@ impl<W: Write> Write for Compressor<W> {
Compressor::Null(w) => w.flush(),
#[cfg(feature = "zstd")]
Compressor::Zstd(w) => w.flush(),
#[cfg(feature = "lz4")]
Compressor::Lz4(w) => w.flush(),
}
}
Expand All @@ -706,7 +706,10 @@ impl<W: Write + Seek> ChunkWriter<W> {
let compression_name = match compression {
#[cfg(feature = "zstd")]
Some(Compression::Zstd) => "zstd",
#[cfg(feature = "lz4")]
Some(Compression::Lz4) => "lz4",
#[cfg(not(any(feature = "zstd", feature = "lz4")))]
Some(_) => unreachable!("`Compression` is an empty enum that cannot be instantiated"),
None => "",
};

Expand All @@ -726,14 +729,16 @@ impl<W: Write + Seek> ChunkWriter<W> {
let compressor = match compression {
#[cfg(feature = "zstd")]
Some(Compression::Zstd) => {
#[allow(unused_mut)]
let mut enc = zstd::Encoder::new(writer, 0)?;
#[cfg(not(target_arch = "wasm32"))]
enc.multithread(num_cpus::get_physical() as u32)?;
Compressor::Zstd(enc)
}
Some(Compression::Lz4) => {
let b = lz4::EncoderBuilder::new();
Compressor::Lz4(b.build(writer)?)
}
#[cfg(feature = "lz4")]
Some(Compression::Lz4) => Compressor::Lz4(lz4_flex::frame::FrameEncoder::new(writer)),
#[cfg(not(any(feature = "zstd", feature = "lz4")))]
Some(_) => unreachable!("`Compression` is an empty enum that cannot be instantiated"),
None => Compressor::Null(writer),
};
let compressor = CountingCrcWriter::new(compressor);
Expand Down
1 change: 1 addition & 0 deletions rust/tests/compression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ fn zstd_round_trip() -> Result<()> {
round_trip(Some(mcap::Compression::Zstd))
}

#[cfg(feature = "lz4")]
#[test]
fn lz4_round_trip() -> Result<()> {
round_trip(Some(mcap::Compression::Lz4))
Expand Down

0 comments on commit 6fd068f

Please sign in to comment.