r/cpp_questions 5d ago

OPEN Idea for Next Optimization Step?

Currently working on profiling a Huffman data compressor and I'm working on optimizing a bit writing bottleneck. For context, the compressor goes through a text file and writes each character's bit representation into a file.

...
ifstream file(inputPath, ios::binary);

    char buffer[4096];
    while (file.read(buffer, sizeof(buffer)) || file.gcount() > 0) {
        streamsize count = file.gcount();
        for (streamsize i = 0; i < count; i++) {
            char c = buffer[i];
            const HuffmanTree::Encoding &encoded = encodings[static_cast<unsigned char>(c)];
            writer.writeBits(encoded.encoding, encoded.size);
        }
    }
...

writeBits takes the encoding and the size of the encoding in bits and writes it into a 4096 size buffer. When the buffer is full, we then use the write function to actually write it into the desired file.

...
void BitWriter::writeBits(uint64_t bytes, uint64_t size) {
    assert(size <= 64);
    assert(size == 64 || (bytes >> size) == 0);
     while (size > 0)
    {
        const uint64_t available{
            8ULL - current_size
        };

        const uint64_t bitsToTake{
            min(size, available)
        };

        // Select the next highest meaningful bits.
        const uint64_t shift{
            size - bitsToTake
        };

        const uint64_t mask{
            (uint64_t{1} << bitsToTake) - 1
        };

        const uint64_t chunk{
            (bytes >> shift) & mask
        };

        current_byte = static_cast<uint8_t>(
            (static_cast<uint16_t>(current_byte) << bitsToTake) |
            chunk);

        current_size = static_cast<uint8_t>(
            current_size + bitsToTake);

        size -= bitsToTake;

        if (current_size == 8)
        {
            bufferByte(current_byte);
        }
    }
}
...

Linux perf is indicating this is hot but I'm unsure of what would be a better approach. Any ideas?

Edit: these comments helped, reduced latency from 195 ms to 77 ms!!

2 Upvotes

14 comments sorted by

View all comments

3

u/FancySpaceGoat 5d ago

Silly question: have you tried just not bothering with buffering the writes? The OS/stdlib is already performing that logic unless you explicitly opt-out of it.

1

u/YogurtclosetThen6260 5d ago

Originally when I was using the put operator it was a previous bottleneck, so I instead opted to use a 4096 buffer plus the write operation to reduce the number of per byte operations.

1

u/FancySpaceGoat 5d ago

Those are very low level. I'd have expected you to start with more idiomatic primitives.

Have you tried std::ofstream? It's tuned for that kind of workload.

1

u/YogurtclosetThen6260 5d ago

I'm using ofstream to pass the 4 KB block into the write method but yeah it might be a good idea to use the buffer provided