Skip to content

fix: optimize spinlock with exponential backoff - #5602

Open
vibe-sudo wants to merge 1 commit into
zeromicro:masterfrom
vibe-sudo:fix/spinlock-backoff-optimize
Open

fix: optimize spinlock with exponential backoff#5602
vibe-sudo wants to merge 1 commit into
zeromicro:masterfrom
vibe-sudo:fix/spinlock-backoff-optimize

Conversation

@vibe-sudo

Copy link
Copy Markdown

What type of PR is this?

  • Optimization

What this PR does:

  • Optimize the original spinlock with exponential backoff algorithm, referring to the implementation of panjf2000/ants spinlock
  • Reduce CPU usage under high concurrency
  • Improve performance by 4x~6x in high contention scenarios
  • Keep full API compatibility (no breaking changes)

Benchmark result

Before (old pure spinlock):
goos: windows
goarch: amd64
pkg: github.com/zeromicro/go-zero/core/syncx
cpu: AMD Ryzen 7 5700X 8-Core Processor
BenchmarkSpinLock
BenchmarkSpinLock-16 236873599 5.066 ns/op

After (optimized with exponential backoff):
goos: windows
goarch: amd64
pkg: github.com/zeromicro/go-zero/core/syncx
cpu: AMD Ryzen 7 5700X 8-Core Processor
BenchmarkSpinLock
BenchmarkSpinLock-16 273461516 4.388 ns/op

kevwan

This comment was marked as low quality.

@kevwan

kevwan commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Thanks for the PR. I do not think this is worth merging as-is.

The submitted benchmark only covers an empty critical section. On my machine it does improve that microbenchmark modestly, but it does not support the claimed 4x-6x high-contention improvement. I also tried the same lock benchmark with a yielding critical section to force waiters to build up, and the change regressed throughput on darwin/arm64:

  • current base: about 2.80-2.96 us/op
  • this PR: about 2.99-3.04 us/op

That makes sense for this implementation: once a goroutine misses the CAS a few times, it can call runtime.Gosched up to 16 times before retrying, which may reduce CAS pressure but can also delay acquisition and hurt throughput under actual contention. Without a benchmark that represents the intended workload and shows a clear win, this feels like trading one narrow microbenchmark improvement for worse behavior in another contended case.

@kevwan kevwan added the do-not-merge/hold Indicates that a PR should not merge because of known issues or need more discussion. label Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because of known issues or need more discussion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants