Skip to content

spektrafilm: tile the Young-van Vliet row pass through local memory - #22825

Merged
TurboGit merged 1 commit into
darktable-org:masterfrom
piratenpanda:yvv_optimize
Oct 29, 2026
Merged

TurboGit merged 1 commit into
darktable-org:masterfrom
piratenpanda:yvv_optimize

Conversation

@piratenpanda

Copy link
Copy Markdown
Contributor

Summary

The YvV row kernels ran one work-item per row, each walking its own row, so neighboring work-items accessed memory a full row apart. The row pass was 1.8–10x slower than the column pass over the same data, and the largest kernel cost of spektrafilm on the GPU.

A work-group of 16x16 items now covers 16 rows and walks them in chunks of 64 columns. Each chunk is loaded and stored by the whole group in contiguous segments through local memory, and one item per row runs the unchanged recursion on the chunk. The arithmetic and its order are untouched, and the output is bit-identical to before. That was checked on all three drivers below, built with and without -cl-fast-relaxed-math, at 6000x4000 and at 6001x4001 (sizes that aren't multiples of the tile size).

Row pass, 6000x4000, median of 10–20 runs, standalone benchmark of both kernel versions:

device / driver float4 before → after float before → after
Apple M1 Pro, rusticl (asahi) 100.2 → 29.5 ms (3.4x) 13.7 → 8.4 ms (1.6x)
RX 6700 XT, rusticl (radeonsi) 8.6 → 6.8 ms (1.3x) 5.4 → 2.0 ms (2.7x)
RX 6700 XT, ROCm 8.9 → 5.7 ms (1.5x) 5.4 → 2.5 ms (2.2x)

The row kernels now require a work-group size of 256 (reqd_work_group_size(16, 16, 1)), which every supported GPU provides.

Checklist

  • I have read CONTRIBUTING.md and the coding style.
  • I have not merged master into the topic branch.
  • The pull request is one logical change, and every commit compiles on its own. You may use git rebase --exec 'cmake --build build master to check this.
  • I ran the relevant tests: unit tests, src/tests/integration/ where the pixelpipe is touched, or darktable-cli as a headless smoke test.
  • New user-visible strings use _(), new preferences are registered in data/darktableconfig.xml.in.
  • A RELEASE_NOTES.md entry was added (only needed if fixing an issue in a release). Do not reference GitHub issues.

AI assistance

Co-created with Claude Opus 5.5

@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

The row kernels now require a work-group size of 256 (reqd_work_group_size(16, 16, 1)), which every supported GPU provides.

Sure about this? We only have the max group size defined by the standard. Couldn't find a ref.

The row kernels ran one work-item per row, each walking its own row, so
neighbouring items accessed memory one row apart, and the row pass was
far slower than the column pass over the same data.

A work-group now covers several rows and walks them in chunks of 64
columns: each chunk is loaded and stored by the whole group in contiguous
segments through local memory, and one item per row runs the unchanged
recursion on the chunk. The work-group is 8 x 16 items, or smaller as
dt_opencl_local_buffer_opt() finds the device's work-group and local
memory limits require. Output is bit-identical for every tile size.

Row pass on 6000x4000, median of 10, built with -cl-fast-relaxed-math,
before -> after:

  device / driver                  float4             float
  Apple M1 Pro, rusticl (asahi)    100.3 -> 22.1 ms   13.8 -> 6.0 ms
  RX 6700 XT, rusticl (radeonsi)     8.8 ->  6.7 ms    5.4 -> 1.5 ms
  RX 6700 XT, ROCm                   9.1 ->  6.3 ms    5.4 -> 1.7 ms
@piratenpanda

Copy link
Copy Markdown
Contributor Author

You're right, the standard only guarantees 1. I dropped reqd_work_group_size: the kernel now takes its tile size from the launch and its local-memory tile as an argument, and the host sizes it with dt_opencl_local_buffer_opt() like bloom does.

I ran a tile sweep on M1 (rusticl) and RX 6700 XT (rusticl and ROCm) that put 8x16 at or near the best on all three; updated numbers are in the commit message. So this is the starting point, shrinking to what the device and kernel allow, CPU fallback if nothing fits. Output is bit-identical for every tile size tried (16x16 down to 1x1, with and without -cl-fast-relaxed-math, including sizes that aren't multiples of the tile).

@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

Yes, that's the correct way. The possible workgroup size depends on what's inside the kernel, locals, ... It even makes a difference if you allow the compiler to re-use valiable like floats/ints ... even making them const can affect that.

@TurboGit TurboGit added this to the 5.8 milestone Oct 29, 2026
@TurboGit TurboGit added priority: medium core features are degraded in a way that is still mostly usable, software stutters scope: performance doing everything the same but faster OpenCL Related to darktable OpenCL code labels Oct 29, 2026

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TurboGit
TurboGit merged commit 25124eb into darktable-org:master Oct 29, 2026
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OpenCL Related to darktable OpenCL code priority: medium core features are degraded in a way that is still mostly usable, software stutters scope: performance doing everything the same but faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants