Use image.copyMemory() for batch thumbnail generation#76979
Use image.copyMemory() for batch thumbnail generation#76979adamsilverstein merged 4 commits intotrunkfrom
Conversation
Decode the source image once via copyMemory() and use thumbnailImage() for each sub-size, instead of re-decoding the source buffer for every thumbnail. This approach was suggested by the wasm-vips maintainer and avoids both re-decoding overhead and the need for intermediate format conversions. The new batchResizeImage() function in the vips package materializes the decoded image in WASM memory, then generates all thumbnails from that single in-memory copy. Results are written directly to the target output format, eliminating separate transcode steps. Falls back to per-thumbnail processing if the batch operation fails.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Performance AnalysisHow it worksThe core optimization is decode once, resize many. Instead of each thumbnail independently decoding the source image from its compressed buffer, we:
Per-operation comparisonWordPress default generates ~7 sub-sizes (thumbnail, medium, medium_large, large, 1536x1536, 2048x2048) plus a scaled version for large images. Here's what happens for 8 sub-sizes:
Where the time savings come from1. Source decoding (biggest win) Each The libvips discussion showed ~30ms per thumbnail with 2. Worker RPC overhead Each RPC call involves serializing args, posting a message to the worker, and receiving the result. With 3. Queue scheduling In the per-thumbnail path, each thumbnail is a separate queue item competing for the 2 concurrent image processing slots ( Format conversion scenario (AVIF → JPEG)This is where the improvement is most dramatic. In the current per-thumbnail flow: With the batch approach: The expensive AVIF encode step is eliminated entirely for sub-sizes when the output format differs. Memory considerations
For very large images (e.g., 10K×10K = ~400MB), this could be significant. The fallback path provides a safety net — if the batch allocation fails (OOM), it falls back to per-thumbnail processing automatically. What's NOT changed
|
|
Size Change: +746 B (+0.01%) Total Size: 7.74 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 0120477. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24294887127
|
| let vipsPromise: Promise< typeof Vips > | undefined; | ||
| let vipsInstance: typeof Vips; | ||
|
|
||
| /** | ||
| * Instantiates and returns a new vips instance. | ||
| * | ||
| * Reuses any existing instance. | ||
| */ | ||
| async function getVips(): Promise< typeof Vips > { | ||
| if ( vipsPromise ) { | ||
| return await vipsPromise; | ||
| if ( vipsInstance ) { | ||
| return vipsInstance; | ||
| } | ||
|
|
||
| vipsPromise = Vips( { | ||
| vipsInstance = await Vips( { | ||
| // Load HEIF dynamic module for HEIF/HEIC and AVIF format support. | ||
| // JXL is omitted as WordPress Core does not currently support it. | ||
| // It can be re-added when Core adds JXL support. | ||
| dynamicLibraries: [ 'vips-heif.wasm' ], | ||
| locateFile: ( fileName: string ) => { | ||
| // WASM files are inlined as base64 data URLs at build time, | ||
| // eliminating the need for separate file downloads and avoiding | ||
| // issues with hosts not serving WASM files with correct MIME types. | ||
| if ( fileName.endsWith( 'vips.wasm' ) ) { | ||
| return VipsModule; | ||
| } else if ( fileName.endsWith( 'vips-heif.wasm' ) ) { | ||
| return VipsHeifModule; | ||
| } | ||
| return fileName; | ||
| }, | ||
| preRun: ( module: EmscriptenModule ) => { | ||
| // https://github.com/kleisauke/wasm-vips/issues/13#issuecomment-1073246828 | ||
| module.setAutoDeleteLater( true ); | ||
| module.setDelayFunction( ( fn: () => void ) => { | ||
| cleanup = fn; | ||
| } ); | ||
| }, | ||
| } ); | ||
|
|
||
| return await vipsPromise; | ||
| return vipsInstance; | ||
| } |
There was a problem hiding this comment.
Just double-checking, is this an intentional revert of #76780? If not, looks like a possible unintended change during rebase?
There was a problem hiding this comment.
It does look like an unintended change, as this should all be transparent to consumers of getVips(). @adamsilverstein : If at all possible, please keep the vipsPromise approach, as it ensures we only ever have a single instance of Vips.
There was a problem hiding this comment.
Oops, that was not intended. Will fix.
andrewserong
left a comment
There was a problem hiding this comment.
I really like this approach! Sounds like we'd also be able to close out the PNG intermediate PR in #76846 👍
It's generally testing well to me and feels much faster than on trunk. I did run into a curious issue with a test .avif file. I've been randomly grabbing images from this repo to use for testing: https://github.com/link-u/avif-sample-images
Interestingly, with this file, everything works correctly: https://raw.githubusercontent.com/link-u/avif-sample-images/refs/heads/master/kimono.rotate270.avif
However with this file, I get an endless spinner in the editor but no errors logged to the console: https://raw.githubusercontent.com/link-u/avif-sample-images/refs/heads/master/fox.profile0.8bpc.yuv420.avif
I imagine the latter might be due to either the 10 bits per color channel, or its YUV color space. I'm not sure it's caused by this PR, but just thought I'd mention it since I encountered the issue while testing. Also not sure if it means if we need to add additional error handling, or whether there's a gap in the kinds of avif files we can parse?
I'm wrapping up before the Easter break, but happy to do further testing next week!
| * @param smartCrop Whether to use smart cropping (i.e. saliency-aware). | ||
| * @return Array of processed results, one per resize config. | ||
| */ | ||
| export async function batchResizeImage( |
There was a problem hiding this comment.
It looks like there's a fair bit of duplication between resizeImage and this batchResizeImage function. Is it possible to split out a common utility function to avoid the duplication? I'm wondering if that could help ensure that the two behave exactly the same.
I'm mostly thinking of the resize and crop logic.
| } else { | ||
| // Fallback: per-thumbnail processing (original approach | ||
| // without batch resize). | ||
| for ( const name of sizesToGenerate ) { |
There was a problem hiding this comment.
In which cases would we expect the batch resize to fail?
There was a problem hiding this comment.
Maybe if some parts of the batch fail, eg a corrupted image is uploaded? Makes me realize we should be sure to add more failure tests where we expect a certain type of failure (probably on the errror handling PR)
There was a problem hiding this comment.
Hrm, interesting. Maybe not for this PR, then, but I'm curious if there's a case where batch will fail but per-thumbnail will succeed. My assumption (e.g. if there's a corrupt image) is that we'd bail entirely rather than trying another approach to procesing/uploading. But not a blocker to go with this!
Mostly my thinking was if this else condition isn't required then we could remove some code 😄
packages/vips/src/index.ts
Outdated
| // Use faster AVIF encoding for sub-sizes. | ||
| if ( 'image/avif' === outputType ) { | ||
| saveOptions.effort = 2; | ||
| } |
There was a problem hiding this comment.
It looks like this isn't included in resizeImage but is in batchResizeImage. Is there a reason to make these different? Again, maybe we could consolidate logic to ensure we don't have variances between the two functions 🤔
There was a problem hiding this comment.
the consolidated version fixes this omission
Thanks for testing and pointing out the issue with that particular image. I will give it a test to see if I can determine why it is failing and if we can fix or just need to better catch the error. |
Fix unintended revert of #76780 during rebase. The promise-based approach ensures only a single Vips instance is ever created, even when multiple callers race on initialization.
Add applyResizeAndCrop() to consolidate the duplicated dimension calculation and crop logic between resizeImage and batchResizeImage. Add buildSaveOptions() to unify save option construction, which also fixes the missing AVIF effort=2 setting in resizeImage.
@andrewserong this image worked fine for me, can you check the response to the initial upload of the image to see if you get an error? I'm going to test more of the images in avif-sample-images to see if any cause issues. |
Perhaps try after a fresh editor load? I am working separately on an issue where uploading many AVIF files seems to crash the editor. See #76707 I see an error in the console though:
|
|
Confirmed in my testing the original Pisa AVIF image processes much more quickly than trunk, this is a huge improvement, maybe x2! All image formats benefit from this improvement, but its most obvious with AVIF since its so slow the begin with. |
|
I do get a consistent failure and console error, and endless spinner with Working on this in the error handling ticket. |
this is addressed in #76707 |
andrewserong
left a comment
There was a problem hiding this comment.
Great work on this, it's testing really nicely for me, and the consolidation updates are looking good to me. Left a couple of tiny nits but nothing major, I think this is in good shape overall.
And anecdotally, this is feeling quite a bit faster in my local env now, great stuff 🎉
Tested with an assortment of avif, jpeg, animated gif, png, and other images and all held up as expected.
LGTM 🚀
| } else { | ||
| // Fallback: per-thumbnail processing (original approach | ||
| // without batch resize). | ||
| for ( const name of sizesToGenerate ) { |
There was a problem hiding this comment.
Hrm, interesting. Maybe not for this PR, then, but I'm curious if there's a case where batch will fail but per-thumbnail will succeed. My assumption (e.g. if there's a corrupt image) is that we'd bail entirely rather than trying another approach to procesing/uploading. But not a blocker to go with this!
Mostly my thinking was if this else condition isn't required then we could remove some code 😄
| // If resize.height is zero, calculate from aspect ratio. | ||
| resize.height = | ||
| resize.height || ( originalHeight / originalWidth ) * resize.width; | ||
|
|
There was a problem hiding this comment.
Tiny nit: this is mutating the resize object. Should we make this immutable by expanding into a new object and performing mutations on that one instead?
There was a problem hiding this comment.
Addressed in 0120477 — now clones the config into a local target object rather than mutating the caller's ImageSizeCrop.
| // Check cancellation between thumbnails. | ||
| if ( ! inProgressOperations.has( id ) ) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Not a blocker, but I'm wondering a tiny bit about what we want to have happen if a cancellation happens between generating thumbnails. Would we want to abandon them all, or ensure completion of thumbnails? (I.e. how atomic do we want to be?)
As a user, if I were attempting to cancel an upload, I wonder what the expectation would be 🤔
(Don't let this comment derail landing this PR by the way, just a thought)
packages/vips/src/index.ts
Outdated
| top = Math.max( 0, top ); | ||
| resize.width = Math.min( image.width, resize.width ); | ||
| resize.height = Math.min( image.height, resize.height ); | ||
| const strOptions = ''; |
There was a problem hiding this comment.
Tiniest of nits, but is this const useful? I guess it's to document the param we're passing newFromBuffer?
Clone the resize config into a local target object rather than mutating the caller's ImageSizeCrop parameter, and drop an unused strOptions constant in batchResizeImage.
|
Filed follow-ups for the two non-blocking questions from the review so they don't get lost:
Both can be picked up alongside the broader error-handling work. |
|
@andrewserong I addressed your final nits and did some final manual testing - everything looks good, going to merge once CI is green. |
|
Nice one, and thanks for capturing the ideas for follow-ups! 🎉 |

Summary
Closes #77248.
copyMemory()call that materializes the decoded image in WASM memorythumbnailImage()(instance method) instead ofthumbnailBuffer()(static method that re-decodes) for each sub-sizeThis approach was suggested by the wasm-vips maintainer and is based on this libvips discussion where
copyMemory()reduced per-thumbnail time from ~2s to ~30ms (60x faster).Alternative approach to #76752 — instead of using a lossless PNG intermediate to avoid expensive re-encoding, this decodes the source once into memory and generates all thumbnails from that single in-memory copy.
How it works
batchResizeImage()in@wordpress/vipsdecodes the source image once withnewFromBuffer()image.copyMemory()to materialize the full image in WASM memorymemImage.thumbnailImage(width, opts)on the in-memory imageTest plan
image_output_formatsto map JPEG→AVIF — verify AVIF thumbnails from JPEG sourceimage_output_formatsto map AVIF→JPEG — verify JPEG thumbnails from AVIF sourcenpm run test:unit -- packages/vips packages/upload-media