net: Image Cache: Spawn setting image keys on thread pool by Narfinger · Pull Request #44483 · servo/servo

Narfinger · 2026-04-24T13:16:07Z

This replaces the fill_key_cache_with_batch_of_keys with dispatch_fill_key_cache_with_batch_of_keys to put it on the already existing thread pool.

fill_key_cache_with_batch_of_keys is called from the ScriptThread and calls set_webrender_image_key which creates an IpcSharedMemory. For large images this can take a long time.
Offloading it away from the ScriptThread means it has more time for important work.

We additionally add some safeguards around ImageCache to prevent us from requesting new ImageKeys while we are processing them.

Testing: WPT tests will probably not find this implementation issue as it is mostly equivalent code except in the case of data races.

github-actions · 2026-04-24T13:30:43Z

🔨 Triggering try run (#24892134692) for Linux (WPT)

github-actions · 2026-04-24T14:01:38Z

Test results for linux-wpt from try job (#24892134692):

Flaky unexpected result (39)

TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
- TIMEOUT [expected PASS] subtest: Fetching a blob URL immediately before revoking it works in <script> tags.
```
Test timed out
```
OK /IndexedDB/idbdatabase_deleteObjectStore.any.html (#43823)
- PASS [expected FAIL] subtest: Deleted object store's name should be removed from database's list. Attempting to use a deleted IDBObjectStore should throw an InvalidStateError

OK /IndexedDB/transaction-create_in_versionchange.any.html (#44200)

FAIL [expected PASS] subtest: Attempt to create new transactions inside a versionchange transaction

assert_array_equals: events expected property 4 to be "open_rq.success: [object IDBDatabase]" but got "complete_count.success: 2" (expected array ["versionchange_add.success: 1", "versionchange_count.success: 0", "versionchange_add2.success: 2", "versionchange_txn.complete", "open_rq.success: [object IDBDatabase]", "complete_count.success: 2", "complete2_get.success: 1"] got ["versionchange_add.success: 1", "versionchange_count.success: 0", "versionchange_add2.success: 2", "versionchange_txn.complete", "complete_count.success: 2", "open_rq.success: [object IDBDatabase]", "complete2_get.success: 1"])

CRASH [expected OK] /_webgl/conformance2/glsl3/sequence-operator-returns-non-constant.html
FAIL [expected PASS] /css/css-backgrounds/background-size-042.html
FAIL [expected PASS] /css/css-backgrounds/border-image-repeat-space-9.html
OK /css/css-cascade/layer-cssom-order-reverse.html (#36094)
- FAIL [expected PASS] subtest: Delete layer invalidates @font-face
```
assert_equals: expected "220px" but got "133px"
```
FAIL [expected PASS] /css/css-ui/appearance-button-001.html
FAIL [expected PASS] /css/css-ui/appearance-searchfield-001.html
FAIL [expected PASS] /css/filter-effects/kernel-unit-length-002.html
CRASH [expected OK] /fetch/api/cors/cors-preflight-not-cors-safelisted.any.html
CRASH [expected OK] /fetch/api/request/request-cache-default.any.worker.html
OK [expected ERROR] /fetch/fetch-later/quota/same-origin-iframe/accumulated-oversized-payload.https.window.html (#41705)
OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
- FAIL [expected PASS] subtest: sec-fetch-storage-access - Cross-site
```
promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
```
OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-origin destination
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-site destination
OK [expected ERROR] /focus/focus-event-after-switching-iframes.sub.html (#40368)
ERROR [expected TIMEOUT] /html/browsers/browsing-the-web/history-traversal/pageswap/pageswap-initial-navigation.html (#40387)
CRASH [expected OK] /html/browsers/the-window-object/window-open-noopener.html?_top
CRASH [expected OK] /html/canvas/element/compositing/2d.composite.solid.source-out.html
CRASH [expected TIMEOUT] /html/canvas/offscreen/layers/2d.layer.non-invertible-matrix.with-render-states.plain-layer.w.html
ERROR [expected OK] /html/canvas/offscreen/text/2d.text.measure.getActualBoundingBox.tentative.html (#43710)
OK /html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/allow-scripts-flag-changing-2.html (#39703)
- FAIL [expected PASS] subtest: Meta refresh of the original iframe is not blocked if moved into a sandboxed iframe
```
uncaught exception: Error: assert_unreached: The iframe into which the meta was moved must not refresh Reached unreachable code
```
OK /html/semantics/embedded-content/media-elements/playing-the-media-resource/loop-from-ended.tentative.html (#33778)
- FAIL [expected PASS] subtest: play() with loop set to true after playback ended
```
this argument is not a finite floating-point value
```

OK /html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-form-submit.html (#32607)

FAIL [expected PASS] subtest: Navigating iframe loading='lazy' before it is loaded: form submit

uncaught exception: Error: assert_equals: expected "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?" but got "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?src"

OK /html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-nav-location-assign.html (#32863)

FAIL [expected PASS] subtest: Navigating iframe loading='lazy' before it is loaded: location.assign

uncaught exception: Error: assert_equals: expected "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?nav" but got "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?src"

TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_other_frame_popup.sub.html (#39702)
- TIMEOUT [expected FAIL] subtest: Sandboxed iframe can not navigate other frame's popup
```
Test timed out
```

OK /html/semantics/scripting-1/the-script-element/execution-timing/077.html (#22139)

FAIL [expected PASS] subtest: adding several types of scripts through the DOM and removing some of them confuses scheduler

assert_array_equals: expected property 1 to be "Script #1 ran" but got "Script #3 ran" (expected array ["Script #2 ran", "Script #1 ran", "Script #3 ran", "Script #4 ran"] got ["Script #2 ran", "Script #3 ran", "Script #4 ran", "Script #1 ran"])

OK /html/semantics/scripting-1/the-script-element/module/choice-of-error-1.html (#44058)
- PASS [expected FAIL] subtest: Parse errors in different files should be reported depending on different roots
TIMEOUT [expected OK] /html/user-activation/navigation-state-reset-sameorigin.html
- TIMEOUT [expected PASS] subtest: Post-navigation state reset.
```
Test timed out
```
OK /html/webappapis/dynamic-markup-insertion/document-write/module-tla-import.html (#42419)
- FAIL [expected PASS] subtest: document.write in an imported module
```
assert_true: onload must be called expected true got false
```
OK /navigation-timing/test-timing-reload.html
- FAIL [expected PASS] subtest: domContentLoadedEventStart is different after the reload.
```
assert_not_equals: domContentLoadedEventStart is different after the reload. got disallowed value 7.53
```
FAIL [expected PASS] /png/apng/fcTL-dispose-previous-final.html (#41837)
TIMEOUT [expected OK] /pointerevents/compat/pointerevent_touch-action_two-finger_interaction.html
- NOTRUN [expected PASS] subtest: touch two-finger pan on 'touch-action: pan-x pan-y'
- NOTRUN [expected FAIL] subtest: touch two-finger pan on 'touch-action: pinch-zoom'
OK /resource-timing/buffer-full-then-increased.html (#44408)
- FAIL [expected PASS] subtest: Test that overflowing the buffer and immediately increasing its limit does not trigger the resourcetimingbufferfull event
```
assert_equals: Number of entries does not match the expected value. expected 3 but got 1
```
OK /sanitizer-api/sanitizer-inert-document.tentative.html (#44273)
- PASS [expected FAIL] subtest: Test whether setHTMLUnsafe loads the image.
OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?06-10 (#37920)
- PASS [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls in report-only mode.
- PASS [expected TIMEOUT] subtest: Navigate a frame via anchor with javascript:-urls w/ default policy in report-only mode.
- FAIL [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in enforcing mode.
```
promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
```
- FAIL [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in report-only mode.
```
promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
```
OK /visual-viewport/resize-event-order.html (#41981)
- PASS [expected FAIL] subtest: Popup: DOMWindow resize fired before VisualViewport.
CRASH [expected OK] /webaudio/the-audio-api/the-mediastreamaudiodestinationnode-interface/closed-audiocontext-construction.html
CRASH [expected ERROR] /workers/Worker-constructor-proto.any.serviceworker.html

Stable unexpected results that are known to be intermittent (18)

TIMEOUT [expected OK] /_mozilla/mozilla/window-resize-event.html (#36741)
- TIMEOUT [expected PASS] subtest: Popup onresize event fires after resizeTo
```
Test timed out
```

OK /_webgl/conformance/textures/misc/texture-upload-size.html (#21770)

FAIL [expected PASS] subtest: WebGL test #45

assert_true: Texture was smaller than the expected size 2x2 expected true got false

FAIL [expected PASS] subtest: WebGL test #47

assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false

FAIL [expected PASS] subtest: WebGL test #49

assert_true: Texture was smaller than the expected size 2x2 expected true got false

FAIL [expected PASS] subtest: WebGL test #51

assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false

PASS [expected FAIL] subtest: WebGL test #53
PASS [expected FAIL] subtest: WebGL test #55
PASS [expected FAIL] subtest: WebGL test #57
PASS [expected FAIL] subtest: WebGL test #59

FAIL [expected PASS] subtest: WebGL test #61

assert_true: Texture was smaller than the expected size 2x2 expected true got false

FAIL [expected PASS] subtest: WebGL test #63

assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false

And 14 more unexpected results...

ERROR /_webgl/conformance2/textures/misc/tex-3d-size-limit.html (#44383)

FAIL [expected PASS] subtest: WebGL test #77

assert_true: successfullyParsed should be true (of type boolean). Was undefined (of type undefined). expected true got false

OK [expected CRASH] /content-security-policy/meta/sandbox-iframe.html (#43478)
OK /css/css-fonts/generic-family-keywords-003.html (#38994)
- PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted cursive (drawing text in a canvas)
- FAIL [expected PASS] subtest: @font-face matching for quoted and unquoted ui-sans-serif (drawing text in a canvas)
```
assert_equals: unquoted ui-sans-serif does not match @font-face rule expected 40 but got 125
```
TIMEOUT /fetch/metadata/generated/css-images.https.sub.tentative.html (#42229)
- FAIL [expected PASS] subtest: content sec-fetch-site - Same-Origin -> Same Origin
```
assert_unreached: Reached unreachable code
```
TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
- TIMEOUT [expected PASS] subtest: background-image sec-fetch-storage-access - Not sent to non-trustworthy same-origin destination
```
Test timed out
```
OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin-fragment.html (#20768)
- FAIL [expected PASS] subtest: Tests that a fragment navigation in the unload handler will not block the initial navigation
```
assert_equals: expected "" but got "#fragment"
```
TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit-button-click.html (#44099)
- TIMEOUT [expected FAIL] subtest: Replace before load, triggered by submitButton.click()
```
Test timed out
```
TIMEOUT /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
- TIMEOUT [expected FAIL] subtest: Element with tabindex should support autofocus
```
Test timed out
```
- NOTRUN [expected PASS] subtest: Non-HTMLElement should not support autofocus
- NOTRUN [expected FAIL] subtest: Host element with delegatesFocus should support autofocus
OK /html/semantics/embedded-content/media-elements/media_fragment_seek.html (#24114)
- FAIL [expected PASS] subtest: Video should seek to time specified in media fragment syntax
```
assert_equals: expected 3 but got 0
```
OK /html/semantics/forms/form-submission-0/jsurl-form-submit.tentative.html (#36489)
- PASS [expected FAIL] subtest: Verifies that form submissions scheduled inside javascript: urls take precedence over the javascript: url's return value.
OK [expected TIMEOUT] /infrastructure/testdriver/click_nested.html (#43887)
- FAIL [expected NOTRUN] subtest: TestDriver click method with multiple windows and nested iframe
```
can't access property "document", child.frames[2] is undefined
```
OK /mixed-content/tentative/autoupgrades/mixed-content-cors.https.sub.html (#41123)
- PASS [expected FAIL] subtest: Cross-Origin video should get upgraded even if CORS is set
OK /navigation-timing/test-navigation-type-reload.html (#33334)
- FAIL [expected PASS] subtest: Reload domComplete > Original domComplete
```
assert_true: Reload domComplete &gt; Original domComplete expected true got false
```
- FAIL [expected PASS] subtest: Reload domContentLoadedEventEnd > Original domContentLoadedEventEnd
```
assert_true: Reload domContentLoadedEventEnd &gt; Original domContentLoadedEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload domContentLoadedEventStart > Original domContentLoadedEventStart
```
assert_true: Reload domContentLoadedEventStart &gt; Original domContentLoadedEventStart expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventEnd > Original loadEventEnd
```
assert_true: Reload loadEventEnd &gt; Original loadEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventStart > Original loadEventStart
```
assert_true: Reload loadEventStart &gt; Original loadEventStart expected true got false
```
OK /resource-timing/test_resource_timing.html (#25720)
- PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (link)
OK /touch-events/single-tap-when-touchend-listener-use-sync-xhr.html (#41175)
- PASS [expected FAIL] subtest: Click event should be fired when touchend opens synchronous XHR
OK /trusted-types/trusted-types-reporting.html (#43737)
- PASS [expected FAIL] subtest: Trusted Type violation report: creating a forbidden-but-not-reported policy.

github-actions · 2026-04-24T14:01:48Z

✨ Try run (#24892134692) succeeded.

mrobinson · 2026-04-24T14:58:02Z

Hrm. If the problem is loading the image, won't this only affect things when images have been waiting for keys? Why not selectively load large images on a separate thread when we detect that they are large enough to warrant this?

Narfinger · 2026-04-24T15:26:13Z

Currently all images (except the first will be waiting for keys, so it should catch almost all.
Images are loaded via get_cached_image_status and then when the image is not ready yet a listener is added with add_listener_with_store. And the pending loads are then notified when the images are ready and assigned keys (in complete_load). So every image that is loaded will be handled via the added call.
So as I understand it, all images (except the first one) will be waiting for keys.

The advantage of doing it like this is that ImageCacheStore is in a Arc<Mutex<_>> and currently it is very nice to reason about it because everything inside does not have inner mutability.

The alternative would be to spawn thread for set_webrender_image_key and complete_load which I feel will make things more complicated.

Narfinger · 2026-05-12T15:10:36Z

@mrobinson any update on this?

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>

jdm · 2026-06-09T17:11:58Z

Here's an alternative proposal:

insert_keys_and_load_images returns a vector of images that should have their webrender keys set in parallel
load_image_with_keycache returns an enum containing the data necessary to call ImageCacheStore::set_key_and_finish_load, where one variant means the key was set synchronously and one variant means we need to spawn a task
we have a heuristic in

servo/components/net/image_cache.rs

Line 562 in de6c787

self.set_key_and_finish_load(pending_image, image_key);

to decide whether to set the key synchronously or in parallel

The thread pool tasks are only responsible for calling set_key_and_finish_load with the provided data. Some callers of load_image_with_keycache already exist in spawned tasks so we can add a flag to force a synchronous operation for those cases.

Narfinger · 2026-06-17T12:13:00Z

I am not sure if I will have time to work on it but I don't quite understand some points you made.

set_keys_and_finish_load needs access to ImageCache to modifying the PendingImageLoads and CompleteLoads so any parallelism would just content with the lock.
Most of the time we will have keys in the keycache so I don't quite understand why not just always spawning the task is the easier solution.

jdm

Ok, I needed some time to convince myself that we weren't overlooking the case where we already have cached keys. In that case we already spawn a task on the thread pool to perform the image decoding and then set the webrender key. With the changes in this PR we just need to ensure that new batches of image keys trigger the same off-thread webrender key creation. This looks like a reasonable way to make that happen.

Narfinger requested a review from gterzian as a code owner April 24, 2026 13:16

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 24, 2026

Narfinger added the T-linux-wpt Do a try run of the WPT label Apr 24, 2026

github-actions Bot removed the T-linux-wpt Do a try run of the WPT label Apr 24, 2026

spawn to different thread

733b39a

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>

Narfinger force-pushed the image-key-offload branch from f85dd47 to 733b39a Compare June 4, 2026 07:56

Narfinger requested a review from jdm June 9, 2026 10:12

jdm added S-awaiting-answer Someone asked a question that requires an answer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 12, 2026

jdm approved these changes Jun 23, 2026

View reviewed changes

jdm added this pull request to the merge queue Jun 23, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 23, 2026

Merged via the queue into servo:main with commit 92b9f02 Jun 23, 2026
33 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 23, 2026

Narfinger deleted the image-key-offload branch June 24, 2026 07:23

Navigation Menu

Uh oh!

Uh oh!

net: Image Cache: Spawn setting image keys on thread pool - #44483

net: Image Cache: Spawn setting image keys on thread pool#44483
jdm merged 1 commit into
servo:mainfrom
Narfinger:image-key-offload

Narfinger commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

mrobinson commented Apr 24, 2026

Uh oh!

Narfinger commented Apr 24, 2026

Uh oh!

Narfinger commented May 12, 2026

Uh oh!

jdm commented Jun 9, 2026

Uh oh!

Narfinger commented Jun 17, 2026 •

edited

Loading

Uh oh!

jdm left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

Narfinger commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

mrobinson commented Apr 24, 2026

Uh oh!

Narfinger commented Apr 24, 2026

Uh oh!

Narfinger commented May 12, 2026

Uh oh!

jdm commented Jun 9, 2026

Uh oh!

Narfinger commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Narfinger commented Jun 17, 2026 •

edited

Loading