Skip to content

Commit 45b64e3

Browse files
clalancetteclaude
andauthored
cpp: align zstd default level and chunk flush behavior with other writers (#1804)
### Changelog The C++ zstd compression defaults were aligned with the rest of the languages by switching to compression level 3, which is slightly slower but provides better compression. ### Docs None. ### Description The C++ writer diverged from the Go, Rust, Python, and TypeScript MCAP writers in two ways that produce noticeably larger zstd files: 1. CompressionLevel::Default mapped to zstd level 1, while every other implementation compresses at libzstd's default of level 3 (Go's SpeedDefault is the equivalent). Change the mapping to 3. 2. chunkSize was treated as a hard cap: the writer flushed the current chunk before writing any message that would push it past the target. All other implementations treat chunkSize as a threshold, flushing only after the chunk meets or exceeds it. Remove the pre-write flush so chunk boundaries match, and update the chunkSize documentation to describe the threshold semantics. On a simulated 10-second robot recording (5 channels, 3750 messages, ~102 MB uncompressed), these two changes together shrink the C++ zstd output from 42.7 MB to 29.3 MB, with chunk boundaries now byte-identical to those produced by the Rust/Go/Python writers. Writes get correspondingly slower (level 3 costs ~35% more CPU than level 1) but remain the fastest of the five implementations on the same workload. The "Message index records" unit test assumed the cap behavior (its second message relied on the pre-write flush to start a new chunk); size each message to cross the threshold on its own instead, which preserves the regression the test guards against. One important compatibility note: while this does change the defaults, it will not affect rosbag2 out-of-the-box since it explicitly chooses uncompressed as its default. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 6fa700b commit 45b64e3

3 files changed

Lines changed: 11 additions & 18 deletions

File tree

cpp/mcap/include/mcap/writer.hpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,12 @@ struct MCAP_PUBLIC McapWriterOptions {
5656
bool noSummary = false;
5757
/**
5858
* @brief Target uncompressed Chunk payload size in bytes. Once a Chunk's
59-
* uncompressed data is about to exceed this size, the Chunk will be
60-
* compressed (if enabled) and written to disk. Note that this is a 'soft'
61-
* ceiling as some Chunks could exceed this size due to either indexing
62-
* data or when a single message is larger than `chunkSize`, in which case,
63-
* the Chunk will contain only this one large message.
59+
* uncompressed data meets or exceeds this size, the Chunk will be
60+
* compressed (if enabled) and written to disk. Note that this is a
61+
* threshold rather than a ceiling: a Chunk grows until the record that
62+
* meets or exceeds this size has been written to it, so Chunks may exceed
63+
* this size by up to one record. This matches the behavior of the other
64+
* MCAP writer implementations (Go, Rust, Python, TypeScript).
6465
* This option is ignored if `noChunking=true`.
6566
*/
6667
uint64_t chunkSize = DefaultChunkSize;

cpp/mcap/include/mcap/writer.inl

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ int ZStdCompressionLevel(CompressionLevel level) {
241241
return -3;
242242
case CompressionLevel::Default:
243243
default:
244-
return 1;
244+
return 3;
245245
case CompressionLevel::Slow:
246246
return 5;
247247
case CompressionLevel::Slowest:
@@ -577,14 +577,7 @@ Status McapWriter::write(const Message& message) {
577577
channelMessageCounts.emplace(message.channelId, 0);
578578
}
579579

580-
// Before writing a message that would overflow the current chunk, close it.
581580
auto* chunkWriter = getChunkWriter();
582-
if (chunkWriter != nullptr && /* Chunked? */
583-
uncompressedSize_ != 0 && /* Current chunk is not empty/new? */
584-
9 + getRecordSize(message) + uncompressedSize_ >= chunkSize_ /* Overflowing? */) {
585-
auto& fileOutput = *output_;
586-
writeChunk(fileOutput, *chunkWriter);
587-
}
588581

589582
// For the chunk-local message index.
590583
const uint64_t messageOffset = uncompressedSize_;

cpp/test/unit_tests.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ TEST_CASE("McapReader::readMessages()", "[reader]") {
552552
/**
553553
* @brief ensures that message index records are only written for the channels present in the
554554
* previous chunk. This test writes two chunks with one message each in separate channels, with
555-
* the second message being large enough to guarantee the current chunk will be written out.
555+
* each message being large enough to meet the chunk size threshold and close its chunk.
556556
* If the writer is working correctly, there will be one message index record after each chunk,
557557
* one for each message.
558558
*/
@@ -574,10 +574,9 @@ TEST_CASE("Message index records", "[writer]") {
574574
writer.addChannel(channel2);
575575

576576
mcap::Message msg;
577-
// First message should not fill first chunk.
578-
WriteMsg(writer, channel1.id, 0, 100, 100, std::vector<std::byte>{20});
579-
// Second message fills current chunk and triggers a new one.
580-
WriteMsg(writer, channel2.id, 0, 200, 200, std::vector<std::byte>{400});
577+
// Each message meets the chunk size threshold, closing its chunk once written.
578+
WriteMsg(writer, channel1.id, 0, 100, 100, std::vector<std::byte>(400));
579+
WriteMsg(writer, channel2.id, 0, 200, 200, std::vector<std::byte>(400));
581580

582581
writer.close();
583582

0 commit comments

Comments
 (0)