Rich-T-kid commented on code in PR #12148:
URL: https://github.com/apache/arrow-rs/pull/12148#discussion_r4529720169
##########
arrow-ipc/src/compression.rs:
##########
@@ -11,18 +11,44 @@ const LENGTH_NO_COMPRESSED_DATA: i64 = -1;
const LENGTH_OF_PREFIX_DATA: i64 = 8;
const DEFAULT_ZSTD_COMPRESSION_LEVEL: i32 = 3;
+/// The lz4 frame block size used unless the caller configures another size
with
+/// [`crate::writer::IpcWriteOptions::try_with_lz4_block_size`].
+pub(crate) const DEFAULT_LZ4_BLOCK_SIZE: usize = LZ4_BLOCK_256_KIB;
+
+const LZ4_BLOCK_64_KIB: usize = 64 * 1024;
+const LZ4_BLOCK_256_KIB: usize = 256 * 1024;
+const LZ4_BLOCK_1_MIB: usize = 1024 * 1024;
+const LZ4_BLOCK_4_MIB: usize = 4 * 1024 * 1024;
+
+/// The lz4 frame block sizes callers may configure, in bytes.
+///
+/// These are the only sizes the LZ4 frame format can represent: the frame
header stores the block
+/// size as a 3 bit code with the values 4 to 7. `lz4_flex` also offers
`BlockSize::Max8MB`, but it
+/// encodes it by setting the header's reserved bit, which the decoder then
rejects, so it cannot
+/// be exposed here.
+pub(crate) const SUPPORTED_LZ4_BLOCK_SIZES: [usize; 4] = [
+ LZ4_BLOCK_64_KIB,
+ LZ4_BLOCK_256_KIB,
+ LZ4_BLOCK_1_MIB,
+ LZ4_BLOCK_4_MIB,
+];
Review Comment:
if these are the only four valid variants then this should be an enum
##########
arrow-ipc/src/compression.rs:
##########
@@ -603,4 +678,106 @@ mod tests {
let prefix: [u8; 8] = output_bytes[..8].try_into().unwrap();
assert_eq!(i64::from_le_bytes(prefix), input_bytes.len() as i64);
}
+
+ /// Reads the block size descriptor code from the lz4 frame header of a
`compress_to_vec`
+ /// buffer: `[8 byte length prefix][4 byte magic][FLG][BD]`, where BD bits
4-6 hold the code.
+ #[cfg(feature = "lz4")]
+ fn lz4_frame_block_size_code(compressed: &[u8]) -> u8 {
+ const BD_OFFSET: usize = super::LENGTH_OF_PREFIX_DATA as usize + 4 + 1;
+ let prefix: [u8; 8] = compressed[..8].try_into().unwrap();
+ assert_ne!(
+ i64::from_le_bytes(prefix),
+ super::LENGTH_NO_COMPRESSED_DATA,
+ "input was stored uncompressed, frame header unavailable"
+ );
+ (compressed[BD_OFFSET] & 0b0111_0000) >> 4
+ }
+
+ #[test]
+ #[cfg(feature = "lz4")]
+ fn test_lz4_block_size_default_is_256kb() {
Review Comment:
I think these test are a overkill for the changes here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
