Skip to content

perf(thread.h): faster spin_mutex under contention; fix arm64 pause - #5564

Merged
lgritz merged 4 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-spinmutex
Oct 19, 2026
Merged

lgritz merged 4 commits into
AcademySoftwareFoundation:mainfrom
lgritz:lg-spinmutex

Conversation

@lgritz

@lgritz lgritz commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

This started as my taking a look at whether our spin_mutex already used all the good techniques discussed in this article that I happened to read last week: https://david.alvarezrosa.com/posts/optimizing-a-spin-lock/

And indeed, there were some improvements to make, primarily in the way we pause and back off when the lock is held by another thread. But also, some bugs were found along the way. Changes in this PR:

  • pause() had no aarch64 branch (only __arm__), so on Apple Silicon and Linux arm64 it was an empty loop. Use isb; the yield hint is effectively a nop on Apple cores and benchmarks badly. Also keep the compiler from fully unrolling a constant pause count, which bloated lock() enough that clang -O3 stopped inlining it.

  • spin_mutex::lock() hinted the branch backwards (!OIIO_UNLIKELY(try_lock())), marking acquisition as the unlikely case. It should have read OIIO_UNLIKELY(!try_lock()), a subtle difference, but worth up to 2x at 16-32 threads!

  • spin_mutex now holds a std::atomic<bool> and waits with a relaxed load instead of reading the atomic_flag through a volatile bool cast (UB, and the reason TSan needed OIIO_THREAD_ALLOW_DCLP=0). Codegen is identical. Note: on MSVC this changes sizeof(spin_mutex) from 4 to 1.

  • Yielding early was the main cost at 8-12 threads. atomic_backoff gets an optional spins count (rounds at the max pause before yielding, default 0 = old behavior, which ImageCache's tile wait still uses). spin_mutex uses atomic_backoff(128, 64), which was the best of the backoff variants benchmarked on both machines below.

spinlock_test, old -> new:

  • M4 Max (16 cores), ns/op: 4 threads 3.8 -> 1.2, 8 threads 26.2 -> 2.5, 12 threads 18.8 -> 3.8, 16 threads 5.0 -> 5.0. Old is faster only when oversubscribed (32 threads 2.5 -> 6.2).
  • Threadripper PRO 5955WX (16 cores / 32 threads, gcc 11), seconds: 8 threads 3.7 -> 2.3, 16 threads 6.3 -> 4.3, 32 threads 10.9 -> 7.5, 48 threads 10.2 -> 8.1, 64 threads 9.2 -> 9.1; less CPU time up to 48. Uncontended cost is unchanged on both. parallel_test task launch rate is up 5-12% on x86. spin_rw_test and testtex are unchanged.

C++20 atomic_flag::test() gives nothing over atomic<bool>::load() (and benchmarked worse on x86), and wait()/notify() is ~4x slower uncontended. Under heavy contention on macOS it also drove __ulock_wait/__ulock_wake into a kernel spinlock timeout panic, so it is deliberately not used.

I have only the spin_mutex change in thread.h in this PR. I will have subsequent follow-on work elsewhere (including ImageCache), that makes further improvements that this work later turned into.

Assisted-by: Claude Code / Claude Opus 5.5

* pause() had no aarch64 branch (only `__arm__`), so on Apple Silicon
  and Linux arm64 it was an empty loop. Use `isb`; the `yield` hint is
  effectively a nop on Apple cores and benchmarks badly. Also keep the
  compiler from fully unrolling a constant pause count, which bloated
  lock() enough that clang -O3 stopped inlining it.

* spin_mutex::lock() hinted the branch backwards
  (`!OIIO_UNLIKELY(try_lock())`), marking acquisition as the unlikely
  case. Worth up to 2x at 16-32 threads.

* spin_mutex now holds a std::atomic<bool> and waits with a relaxed load
  instead of reading the atomic_flag through a volatile bool cast (UB,
  and the reason TSan needed OIIO_THREAD_ALLOW_DCLP=0). Codegen is
  identical. Note: on MSVC this changes sizeof(spin_mutex) from 4 to 1.

* Yielding early was the main cost at 8-12 threads. atomic_backoff gets
  an optional `spins` count (rounds at the max pause before yielding,
  default 0 = old behavior, which ImageCache's tile wait still uses).
  spin_mutex uses atomic_backoff(128, 64), which was the best of the
  backoff variants benchmarked on both machines below.

spinlock_test, old -> new:
- M4 Max (16 cores), ns/op: 4 threads 3.8 -> 1.2, 8 threads 26.2 -> 2.5,
  12 threads 18.8 -> 3.8, 16 threads 5.0 -> 5.0. Old is faster only
  when oversubscribed (32 threads 2.5 -> 6.2).
- Threadripper PRO 5955WX (16 cores / 32 threads, gcc 11), seconds:
  8 threads 3.7 -> 2.3, 16 threads 6.3 -> 4.3, 32 threads 10.9 -> 7.5,
  48 threads 10.2 -> 8.1, 64 threads 9.2 -> 9.1; less CPU time up to 48.
Uncontended cost is unchanged on both. parallel_test task launch rate
is up 5-12% on x86. spin_rw_test and testtex are unchanged.

C++20 atomic_flag::test() gives nothing over atomic<bool>::load() (and
benchmarked worse on x86), and wait()/notify() is ~4x slower
uncontended. Under heavy contention on macOS it also drove
__ulock_wait/__ulock_wake into a kernel spinlock timeout panic, so it is
deliberately not used.

Assisted-by: Claude Code / Claude Opus 5.5

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>

@jinhgkim jinhgkim left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Spent most of my time learning the subject... and it's pretty cool! Everything makes sense here. I just left one suggestion and a question.

for (int i = 0; i < delay; ++i)
__asm__ __volatile__("isb" ::: "memory");

#elif defined(__GNUC__) && (defined(__arm__) || defined(__s390__))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, if unrolling bloats every inlined lock, why isn't OIIO_PAUSE_NOUNROLL used here too?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Correct. Refactor of this function coming...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated with your suggestion

Comment thread src/include/OpenImageIO/thread.h Outdated
lgritz and others added 2 commits October 16, 2026 17:28
Co-authored-by: Jinnie Kim <jinhgkim@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
@jinhgkim

Copy link
Copy Markdown
Collaborator

8 threads 26.2 -> 2.5, 12 threads 18.8 -> 3.8

Is this true by the way? The speedup here is crazy. On my machine (M5 Pro chip), it's actually slightly slower (8 threads 10.6 -> 12.6, 12 threads 18.2 -> 19.0), and I don't know why... (I've been having a long chat with Claude but still can't figure it out.) The other thread counts match yours (4 threads 3x faster, 16 the same, 32 ~2.7x slower), so it's just 8 and 12 that differ.

@lgritz

lgritz commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

8 threads 26.2 -> 2.5, 12 threads 18.8 -> 3.8

Is this true by the way? The speedup here is crazy. On my machine (M5 Pro chip),

Do you know what your core counts are?

I reran the benchmarks on my M4 Max laptop (12 performance cores and 4 efficiency cores). And got a detailed Claude comparison (it's great at writing a script to kick off a bunch of N-way comparison benchmarks that are more tedious than I would want to do by hand), reproduced below. I also had (not repeated today, but done when I did the original work) ran tests on my work machine (Linux x86_64 64 cores) to verify that nothing bad happened there).


The PR is a big win at 2-12 threads on the M4, but it's about 2x slower than before at 16 threads (all cores) in spinlock_test. Times are best of 3-5 trials of 200M lock/unlock cycles; each row is consistent across two alternating wedges plus a separate 8T/16T rerun.

threads before (a1d18bd) after (lg-spinmutex) after vs before
uncontended 0.9 ns 0.8 ns same
1 0.2 s 0.2 s same
2 0.3-0.4 s 0.2 s 1.5-2x faster
4 0.9 s 0.3 s 3x faster
8 4.7-4.9 s 0.5 s 9.5x faster
12 3.0 s 0.7 s 4.3x faster
16 0.7-0.8 s 1.3-1.4 s about 2x slower
  • 8T, as you asked: 4.7-4.9 s before vs 0.5 s after, about 9.5x faster, with small spread between trials (0.01-0.02 s after).
  • The review commits didn't cause the 16T result. "Rewrite pause loop..." produces the same isb loop on aarch64. A build with the thread.h from lg-spinlock also gives 1.4 s at 16T.
  • The old code's curve isn't monotonic. It collapses at 8-12 threads, then recovers at 16. Its pause loop was empty on aarch64, so waiting threads yielded almost at once. With every core busy, that apparently helps.
  • The new code is predictable. Time rises steadily with thread count, and its worst case (1.4 s at 16T) is still well below the old one's worst (4.9 s at 8T).

Why 16T regresses (my guess, not measured): the M4 Max has 12 performance and 4 efficiency cores. If the lock holder lands on a slower efficiency core, the 15 threads spinning on isb for about 128 rounds before yielding may slow it down while it holds the lock.

The original M4 study's own benchmark showed 16T improving (21.3 -> 12.6 ns on a tiny critical section). That benchmark had a different shape, and spinlock_test exposes this case that it didn't. goat0225 [the work machine I tested on -- LG] showed no 16T regression with this test (4.4x faster at 16T).

@jinhgkim

Copy link
Copy Markdown
Collaborator

Do you know what your core counts are?

6 super cores + 12 performance cores

@lgritz

lgritz commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

So I have 12 perf + 4 efficiency, and am ok at <= 12 cores but hit a performance cliff at 16.

You have 6 super + 12 perf, and hit a performance cliff at 8 cores?

That sounds suspiciously like as soon as the job spills from using only the machine's best core type to being a mix of that and a second core type, things aren't as smooth. I wonder if these Apple computers have some odd bus synchronization properties when you're mixing different core type accessing the same memory locations?

@jinhgkim

Copy link
Copy Markdown
Collaborator

Yesss, mine is fine at <= 6

threads base PR PR vs base
4 14.7 5.1 2.9x faster
5 19.1 5.5 3.5x faster
6 27.3 6.1 4.5x faster
7 11.1 9.6 1.15x faster
8 11.0 12.7 1.15x slower

@jinhgkim

Copy link
Copy Markdown
Collaborator

Did some experiments mixing core types with the help of Claude

where the 6 threads ran Base (ns/op) PR (ns/op)
all on Super 26.4 6.1
mostly on P, at ~2.3 GHz (it still ran 1 thread on S) 21.3 16.1
3 on Super + 3 on P (P at ~2.3 GHz) 33.8 14.7

Mixing core types doesn't hurt the PR (14.7 is between all-Super and all-P), and it's still about 2.3x faster than Base.

Claude says the PR is slower at 8T and 12T because Base suddenly gets faster when the thread count goes above the best core type's count.

threads Base PR
6 25.5 6.2
7 11.4 9.2
8 11.1 11.8
12 16.8 17.0

Also a summary of Claude's conclusion:

The reason seems to be Base's yields. Base's waiting threads call sched_yield() constantly. Once there are more threads than top-tier cores, some threads have to run on lower-tier cores, off their preferred core type, and that likely makes each yield on the top-tier cores go through the full scheduler to check whether those threads should be moved back (see XNU's sched_edge_thread_should_yield()). At 7T, Base's yields on the top-tier cores take about 0.96 µs instead of 0.23 µs, so its waiting threads retry the lock far less often and get in the way of the thread holding the lock less. The PR's waiters almost never yield, so they don't get this effect.

I think this makes sense, and it seems to fit the benchmarks on both Macs. What do you think?

@lgritz

lgritz commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Yeah, that makes sense to me. There's a sense in which the benchmark isn't quite measuring the same thing once cores are oversubscribed or when they are using different core types, versus when each thread gets a core and they're all the same kind. So those charts are really mixing two different situations and pretending it's a simple linear relationship.

The spin_mutex is meant for situations where contention is reasonably low and the lock is held for a very short time. Cranking up the contention as high as we can make it for the benchmarks -- literally doing nothing outside the critical path, just having all threads hit the lock incessantly -- is pushing them out of the use case they were designed for or that they are likely to encounter in the real world.

I think the bottom line is that the bug fixes and the new backoff makes the performance curve much more predictable, and dramatically better when all threads get cores (which is the usual situation, although they are slightly slower than before when in the odd situation of mixed cores or not enough cores) and in the lower-contention domain they were designed for.

I think that's a fair tradeoff.

@lgritz
lgritz merged commit 68b381b into AcademySoftwareFoundation:main Oct 19, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants