Skip to content

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 15, 2025

The atomic_wait benchmarks are great, but they tend to overload the system they're running on. For that reason, we can't run them on our CI infrastructure on a regular basis.

Instead of removing them, make them unsupported outside of dry-running, which allows keeping the benchmarks around and ensuring they don't rot, but doesn't run them along with the other benchmarks. If we need to investigate atomic_wait performance, it's trivial to mark the benchmark as supported and run it for local investigations.

This is an alternative to #158289.

The atomic_wait benchmarks are great, but they tend to overload the
system they're running on. For that reason, we can't run them on our
CI infrastructure on a regular basis.

Instead of removing them, make them unsupported outside of dry-running,
which allows keeping the benchmarks around and ensuring they don't rot,
but doesn't run them along with the other benchmarks. If we need to
investigate atomic_wait performance, it's trivial to mark the benchmark
as supported and run it for local investigations.

This is an alternative to llvm#158289.
@ldionne ldionne requested a review from a team as a code owner September 15, 2025 12:56
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 15, 2025

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

The atomic_wait benchmarks are great, but they tend to overload the system they're running on. For that reason, we can't run them on our CI infrastructure on a regular basis.

Instead of removing them, make them unsupported outside of dry-running, which allows keeping the benchmarks around and ensuring they don't rot, but doesn't run them along with the other benchmarks. If we need to investigate atomic_wait performance, it's trivial to mark the benchmark as supported and run it for local investigations.

This is an alternative to #158289.


Full diff: https://github.com/llvm/llvm-project/pull/158631.diff

4 Files Affected:

  • (modified) libcxx/test/benchmarks/atomic_wait_1_waiter_1_notifier.bench.cpp (+4)
  • (modified) libcxx/test/benchmarks/atomic_wait_N_waiter_N_notifier.bench.cpp (+4)
  • (modified) libcxx/test/benchmarks/atomic_wait_multi_waiter_1_notifier.bench.cpp (+4)
  • (modified) libcxx/test/benchmarks/atomic_wait_vs_mutex_lock.bench.cpp (+4)
diff --git a/libcxx/test/benchmarks/atomic_wait_1_waiter_1_notifier.bench.cpp b/libcxx/test/benchmarks/atomic_wait_1_waiter_1_notifier.bench.cpp
index c3d7e6511925d..5c82b8e8e9af6 100644
--- a/libcxx/test/benchmarks/atomic_wait_1_waiter_1_notifier.bench.cpp
+++ b/libcxx/test/benchmarks/atomic_wait_1_waiter_1_notifier.bench.cpp
@@ -8,6 +8,10 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
+// This benchmark is very expensive and we don't want to run it on a regular basis,
+// only to ensure the code doesn't rot.
+// REQUIRES: enable-benchmarks=dry-run
+
 #include "atomic_wait_helper.h"
 
 #include <atomic>
diff --git a/libcxx/test/benchmarks/atomic_wait_N_waiter_N_notifier.bench.cpp b/libcxx/test/benchmarks/atomic_wait_N_waiter_N_notifier.bench.cpp
index d9b9aa212f602..4d6547418e767 100644
--- a/libcxx/test/benchmarks/atomic_wait_N_waiter_N_notifier.bench.cpp
+++ b/libcxx/test/benchmarks/atomic_wait_N_waiter_N_notifier.bench.cpp
@@ -8,6 +8,10 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
+// This benchmark is very expensive and we don't want to run it on a regular basis,
+// only to ensure the code doesn't rot.
+// REQUIRES: enable-benchmarks=dry-run
+
 #include "atomic_wait_helper.h"
 
 #include <atomic>
diff --git a/libcxx/test/benchmarks/atomic_wait_multi_waiter_1_notifier.bench.cpp b/libcxx/test/benchmarks/atomic_wait_multi_waiter_1_notifier.bench.cpp
index a14a6a2ad9c98..f8288cb4c8020 100644
--- a/libcxx/test/benchmarks/atomic_wait_multi_waiter_1_notifier.bench.cpp
+++ b/libcxx/test/benchmarks/atomic_wait_multi_waiter_1_notifier.bench.cpp
@@ -8,6 +8,10 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
+// This benchmark is very expensive and we don't want to run it on a regular basis,
+// only to ensure the code doesn't rot.
+// REQUIRES: enable-benchmarks=dry-run
+
 #include "atomic_wait_helper.h"
 
 #include <atomic>
diff --git a/libcxx/test/benchmarks/atomic_wait_vs_mutex_lock.bench.cpp b/libcxx/test/benchmarks/atomic_wait_vs_mutex_lock.bench.cpp
index a554c721df017..9849ffa7e038d 100644
--- a/libcxx/test/benchmarks/atomic_wait_vs_mutex_lock.bench.cpp
+++ b/libcxx/test/benchmarks/atomic_wait_vs_mutex_lock.bench.cpp
@@ -8,6 +8,10 @@
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 
+// This benchmark is very expensive and we don't want to run it on a regular basis,
+// only to ensure the code doesn't rot.
+// REQUIRES: enable-benchmarks=dry-run
+
 #include <atomic>
 #include <cstdint>
 #include <mutex>

@ldionne ldionne requested a review from huixie90 September 18, 2025 15:51
@ldionne
Copy link
Member Author

ldionne commented Sep 19, 2025

Merging based on the acceptance of #158289

@ldionne ldionne merged commit 815b164 into llvm:main Sep 19, 2025
76 of 82 checks passed
@ldionne ldionne deleted the review/disable-atomic-wait-benchmarks branch September 19, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants