Pre-reserve rowlen in UnfilteringBuffer

[197g]

I wonder if (maybe even as a separate PR, or at least a separate commit) it would help to change how UnfilteringBuffer is initialized. In particular, I wonder if things would be simplified if UnfilteringBuffer would always have room for 2 mutable rows (2 times rowlen bytes). IMHO this may be beneficial in the following ways:

• This way prev_row always exists (initially filled with zeros) and we don’t need to worry about special-casing unfiltering of the first row.
• This way there is no need for shifting/shuffling things around in fn unfilter_ahead_row / fn unfilter_curr_row_in_scratch_buffer, because the "scratch buffer" is always / already present at the beginning of the buffer.

WDYT?

Originally posted by @anforowicz in #640 (comment)

[fintelia]

Special-casing the first row is meant as a performance optimization. Using the 'sub' filter instead of 'paeth' should be faster and we don't have to fill an entire row of bytes with zeros. Though I don't know whether that case is common enough to matter.

[anforowicz]

Special-casing the first row is meant as a performance optimization. Using the 'sub' filter instead of 'paeth' should be faster

Ack.

and we don't have to fill an entire row of bytes with zeros.

I am not sure how important this part is. We already have to initialize UnfilterBuf::buffer / UnfilterBuf::data_stream with zeros. The change proposed here would mean that we need to initialize more bytes, but I assume that this should have very little impact on overall performance, because:

• On some (most?) target platforms, alloc_zeroed should be able to directly ask the OS for new zeroed-out memory pages. (Gemini says that this includes Linux, macOS, and Windows, but I didn't confirm this.)
• Allocating the buffer should be a small part of the overall runtime (dwarfed by the actual unfiltering and decompression).

Though I don't know whether that case is common enough to matter.

In my top500 corpus from 2023, I see that:

• 5.8% of images have a height smaller than 16
• 22.7% of images have a height smaller than 18

Unfiltering of those images would be indeed slowed down by 1/16 or more.

---

I think the above doesn't immediately disqualify the idea to pre-reserve rowlen in UnfilteringBuffer. And in general it is hard to weigh 1) code simplification/maintainability improvements vs 2) runtime performance improvements. But we probably should use the hierarchy of constituents - code simplification only benefits authors/maintainers, but performance benefits all clients of the library.

And there is also a question of if unconditionally pre-allocating is ok for really big rowlens. (10 biggest widths in my top500 corpus are 2607-4501 pixels, so potentially x4 or x8 as many bytes.)

---

So given above, maybe this idea should be shelved for now?

[Shnatsel]

On some (most?) target platforms, alloc_zeroed should be able to directly ask the OS for new zeroed-out memory pages. (Gemini says that this includes Linux, macOS, and Windows, but I didn't confirm this.)

This is true only for certain allocators (e.g. on Linux the glibc allocator does that, while mimalloc usually doesn't) and only if the memory is being requested from the OS, as opposed to reused from the userspace allocator.

You mostly see this fast path when the memory is being allocated for the first time, but in a long-lived program that frees and allocates memory a lot you'll see your alloc_zeroed mostly backed by memset()s.