JakeDern commented on PR #10122: URL: https://github.com/apache/arrow-rs/pull/10122#issuecomment-4692241483
> a possible minor/medium optimization for the uncompressed case came to mind & I remembered the current [benchmarks](https://github.com/apache/arrow-rs/blob/main/arrow-ipc/benches/ipc_writer.rs) dont cover this. @JakeDern what do you think about adding an identical case to > > ``` > group.bench_function("StreamWriter/write_10/zstd", |b| { > let batch = create_batch(8192, true); > let mut buffer = Vec::with_capacity(2 * 1024 * 1024); > b.iter(move || { > buffer.clear(); > let options = IpcWriteOptions::default() > .try_with_compression(Some(CompressionType::ZSTD)) > .unwrap(); > let mut writer = > StreamWriter::try_new_with_options(&mut buffer, batch.schema().as_ref(), options) > .unwrap(); > for _ in 0..10 { > writer.write(&batch).unwrap(); > } > writer.finish().unwrap(); > }) > }); > ``` > > but with `try_with_compression` set to None. It may make sense to actually refactor the `StreamWriter` benchmarks to iterate over the possible compression types. I would add it but that would come with possible merge conflicts with this change. If not thats fine I can submit a PR. I think this benchmark is covering that case: https://github.com/apache/arrow-rs/blob/c4a831a1c81be3c20dff510d73954fa6712d90d5/arrow-ipc/benches/ipc_writer.rs#L29-L41 The default `StreamWriter::try_new` uses the default `IpcWriteOptions` which sets `batch_compression_type: None`. Let me know if I'm misunderstanding though. -- 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]
