[akx]

Closes #9220 (supersedes it).

This is an extension of my comment on #9220 (comment):

• Encoders, both C and Python, now talk a new optimal_bufsize property, the number of bytes needed in a byte buffer to encompass the whole encoding.
• Image.tobytes() now checks the optimal bufsize after it initializes the encoder, and passes it through if known; otherwise, the same old logic (the larger of MAXBLOCK, or the memory required to encode a single 8-bit RGBA row) is used.
  • With an exact buffer size passed in, RAW encoding only requires a single allocation, not N allocations of the chunk size plus the accumulation to join into a buffer (which is also work). In memory-constrained situations, the last would have failed anyway, only unfortunately after all of the encoding work had been done.

tobytes() is being used in surprising places around Pillow (see the changelog fragment), so this should have pleasant effects all around...

[akx]

Oh, and for proof, with this reproducer gist:

(main) $ uv pip install -q -e . && time uv run rss_repro.py
revision: 23cbae1d6fbf017855ea40d2421a33fb2e503899
build:    no optimal_bufsize -- MAXBLOCK chunks and a join
platform: darwin, python 3.14.6, mode RGB

        size       result     peak RSS   over baseline   ratio
--------------------------------------------------------------
   2000x2000     11.4 MiB     64.4 MiB        26.6 MiB   2.33x
   4000x4000     45.8 MiB    191.2 MiB       107.9 MiB   2.36x
   8000x8000    183.1 MiB    700.4 MiB       433.2 MiB   2.37x
   20000x500     28.6 MiB    127.7 MiB        67.1 MiB   2.34x

________________________________________________________
Executed in    1.69 secs    fish           external
   usr time    1.13 secs    0.11 millis    1.13 secs
   sys time    0.51 secs    3.88 millis    0.51 secs

and

(tobytes-buf-size) $ uv pip install -q -e . && time uv run rss_repro.py
revision: de42638fe246080aefb616f2dfbb9a12d125c0db
build:    optimal_bufsize present, reports 768 for (16, 16) RGB
platform: darwin, python 3.14.6, mode RGB

        size       result     peak RSS   over baseline   ratio
--------------------------------------------------------------
   2000x2000     11.4 MiB     49.1 MiB        11.6 MiB   1.01x
   4000x4000     45.8 MiB    129.2 MiB        45.7 MiB   1.00x
   8000x8000    183.1 MiB    450.5 MiB       183.7 MiB   1.00x
   20000x500     28.6 MiB     89.7 MiB        28.8 MiB   1.01x

________________________________________________________
Executed in    1.52 secs    fish           external
   usr time    1.08 secs    0.11 millis    1.08 secs
   sys time    0.40 secs    3.88 millis    0.39 secs

[codspeed]

Merging this PR will improve performance by ×2.9

3 warnings

⚠️ 2 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ 21 benchmarks with a significant change ran on different hardware

Their base and head results were measured on machines with a different CPU, core count or memory, so the changes might not come from the code. Run base and head on the same runner type.

Learn more about Macro Runners

See affected benchmarks

⚠️ 21 benchmarks with a significant change ran in a different software environment

Their base and head results were measured with a different OS or toolchain, such as compiler, runtime or library versions, so the changes might come from this difference rather than from the code. Open an affected benchmark to see what changed.

See affected benchmarks

⚡ 21 improved benchmarks
✅ 647 untouched benchmarks
⏩ 352 skipped benchmarks¹

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
⚡	test_save[1237x811-BMP-RGB]	34 ms	9.2 ms	×3.7
⚡	test_pack[1237x811-RGBA]	27.1 ms	7.5 ms	×3.6
⚡	test_equality[1237x811-RGBA-inverted]	53.5 ms	14.9 ms	×3.6
⚡	test_pack[1237x811-L]	6.7 ms	2 ms	×3.4
⚡	test_equality[1237x811-I;16-inverted]	25.1 ms	7.6 ms	×3.3
⚡	test_equality[1237x811-L-inverted]	12.7 ms	3.9 ms	×3.3
⚡	test_equality[1237x811-RGBA-one-pixel]	55.6 ms	17.2 ms	×3.2
⚡	test_equality[1237x811-I;16-one-pixel]	25.7 ms	8.1 ms	×3.2
⚡	test_equality[1237x811-L-one-pixel]	13 ms	4.1 ms	×3.2
⚡	test_equality[1237x811-I;16-equal]	26.4 ms	8.5 ms	×3.1
⚡	test_equality[1237x811-L-equal]	13.2 ms	4.3 ms	×3
⚡	test_equality[1237x811-RGBA-equal]	59.2 ms	19.5 ms	×3
⚡	test_pack[1237x811-LA]	15.2 ms	5.8 ms	×2.6
⚡	test_equality[1237x811-LA-inverted]	29.1 ms	11.4 ms	×2.5
⚡	test_save[1237x811-TIFF-RGB]	40 ms	15.9 ms	×2.5
⚡	test_equality[1237x811-LA-one-pixel]	30.2 ms	12.6 ms	×2.4
⚡	test_pack[1237x811-RGB]	25.7 ms	11 ms	×2.3
⚡	test_equality[1237x811-LA-equal]	31.3 ms	13.5 ms	×2.3
⚡	test_equality[1237x811-RGB-inverted]	49.3 ms	21.9 ms	×2.2
⚡	test_equality[1237x811-RGB-one-pixel]	51.1 ms	23.6 ms	×2.2
...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing akx:tobytes-buf-size (c58a8c8) with main (0a29000)}

Footnotes

1. 352 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[radarhere]

This isn't referring to an existing PR, right? You're presuming it will happen?

[akx]

No existing PR, it's just a note for a future implementer, be that me or someone else 😅

[radarhere]

I'd like the first attempt of this to be a PR to #9938 instead, just to avoid multiple PRs here about the same thing. It would be nicer if the community could agree on a solution to be reviewed, rather than having two competing PRs to be reviewed.

If a PR to that fork goes unanswered for a time, then we can consider something else.

[akx]

A PR to what, sorry?

[radarhere]

Oh, sorry, wrong link. #9220

[akx]

Yeah, this is basically a rebased rework of that year-old rebase-requiring PR that takes care of my comment (that went unanswered by the community) that the bufsize should only be grown for the raw encoder, in a generic way that lets an encoder tell the machinery about its optimal bufsize rather than the machinery needing to know about specifics.

I'd say this PR should remain open if the other one may, too...

[aclark4life]

@hugovk when merging please add this to the squash commit message so @lgeiger is credited for #9220: Co-authored-by: Lukas Geiger 13285808+lgeiger@users.noreply.github.com

[akx]

@aclark4life Thanks! ❤️ I rebased for cleanliness and added @lgeiger's Co-authored-by to b1bbc1d too.

[lgeiger]

Great! thanks for pushing this over the finishing line

[akx]

@aclark4life Thanks, rebased, and adjusted the changelog :)

[aclark4life]

LGTM

[radarhere]

This is only ever used in Image.py, before the call to encode(), so couldn't you just always use state->bytes?

[akx]

I could, but then I'd want to document that the value returned here is correct only before calling encode(), which sounds worse?

[radarhere]

...how about if the values weren't swapped after the first call? I've pushed a commit.

[radarhere]

I've pushed some commits. The most significant of which is that optimal_bufsize wasn't actually being used in ImageFile after all? Only in Image's tobytes()?

[akx]

@radarhere Thanks! Yeah, ImageFile wasn't actually in the scope of this PR (hence this PR's title) since it wasn't in the scope of the original #9220 either 😄

The title of the PR, as well as changelogs, might need to be updated now that this is no longer only about tobytes().

[radarhere]

It seemed weird to talk about adding an extra property to the encoders… but not actually use it for the most common encoding operation?

Hopefully things are looking good now. I've updated the title. Feel free to update the release notes.

[akx]

It seemed weird to talk about adding an extra property to the encoders… but not actually use it for the most common encoding operation?

I think we might actually not want to use this for save(), and I would prefer to pull e8c00b9 out of this PR. It will increase peak RSS for save operations for formats that are backed by the raw encoder (BMP, PPM, TGA, IM, Palm, ICO, etc) -- previously, those would stream blocks onto disk (or wherever the data was being saved to); with e8c00b9, they encode into memory, then do a very large single write.

[akx]

7aadb12 and c58a8c8 look OK to me though I don't think they should be in this PR (since they're unrelated to the task at hand).

[radarhere]

7aadb12 and c58a8c8 look OK to me though I don't think they should be in this PR (since they're unrelated to the task at hand).

I've created #10424 with the first commit. I don't know what you were trying to say with the second commit, because that's changing a test file that this PR adds.

I think we might actually not want to use this for save(), and I would prefer to pull e8c00b9 out of this PR. It will increase peak RSS for save operations for formats that are backed by the raw encoder (BMP, PPM, TGA, IM, Palm, ICO, etc) -- previously, those would stream blocks onto disk (or wherever the data was being saved to); with e8c00b9, they encode into memory, then do a very large single write.

1. What about this comment?

Pillow/src/PIL/ImageFile.py

Lines 681 to 684 in 56f0ece

	# FIXME: make MAXBLOCK a configuration parameter
	# It would be great if we could have the encoder specify what it needs
	# But, it would need at least the image size in most cases. RawEncode is
	# a tricky case.

2. Saving doesn't necessarily put images onto disk. They are some written to BytesIO() or similar.
3. If this isn't for saving images, then it seems strange to be doing all this quite so generically. I strongly suspect that tobytes() is used with the raw encoder far more than any other encoder. I don't think the property needs to be mentioned explicitly in the release notes, and maybe not even included in PyEncoder.

[akx]

Re c58a8c8: sorry, had a brain-frazzled-fart, unrelated.

1. What about this comment?

They're two different comments according to annotate. The FIXME is effbot's, from 2006 or before. Not sure what it refers to -- MAXBLOCK is configurable and documented as such. Should maybe get rid of it?

For the rest, by my reading libjpeg apparently (sometimes) requires a single buffer large enough to store the entire encoded image, and that comment was added originally in 2013 by @wiredfool in 2a743c9 when that became a thing. The JPEG encoder could be changed to use optimal_bufsize, sure (though in that case it's required_bufsize). Looks like #10124 was the last commit to bump the JPEG bufsize.

But for the general case, by and large encoders are happy to work with smaller buffers.

2. Saving doesn't necessarily put images onto disk. They are some written to BytesIO() or similar.

Yep, I'm aware, and that gets even worse 😅... BytesIO.write() writes into its own internal buffer, so optimal_bufsize in save() would still mean double the peak RSS there -- the encoder encodes into a buffer, then that buffer is passed to BytesIO, which grows its internal buffer to match that, copies the data, and the original buffer is only discarded afterwards. That's analogous to what happens in tobytes() before this PR.

I measured locally, and on this branch (with optimal_bufsize used), 6000x6000 BMPs to BytesIOs take double the peak RSS (105 MiB -> 208 MiB), are slightly slower (25 to 33 msec) likely due to a big allocation being slower (we saw slow big allocations being an issue in #9933)... and for a real file, the peak RSS situation becomes direr (as suspected): 1.7 MiB to 105 MiB (and some slowdown).

Users can, as before, tweak MAXBLOCK to their liking for some more performance vs. memory use.

I strongly suspect that tobytes() is used with the raw encoder far more than any other encoder.

Me too.

If this isn't for saving images, then it seems strange to be doing all this quite so generically.

Only the encoder knows the optimal bufsize (state->bits is initialized based on the rawmode and not readily accessible from Python land), so the property would need to be an encoder's.

I don't think the property needs to be mentioned explicitly in the release notes,

That's up for discussion. If you'd rather it wasn't mentioned explicitly, I'll adjust.

and maybe not even included in PyEncoder.

I think it does belong there, so the protocol of it existing is explicit and available. I hope all third-party Python-implemented encoders extend from PyEncoder.