Skip to content

libc: re-enable benchmarks in post submit ci #119789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nickdesaulniers opened this issue Dec 12, 2024 · 5 comments
Open

libc: re-enable benchmarks in post submit ci #119789

nickdesaulniers opened this issue Dec 12, 2024 · 5 comments

Comments

@nickdesaulniers
Copy link
Member

As a result of

llvm/llvm-zorg#325
llvm/llvm-zorg#342

The cmake option -DLIBC_INCLUDE_BENCHMARKS=ON now fails to build when using llvm-projects/runtimes/ as the cmake root.

CMake Error at /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/benchmarks/CMakeLists.txt:35 (add_executable):
  Target "libc-benchmark-test" links to target "benchmark::benchmark" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/benchmarks/CMakeLists.txt:111 (add_libc_benchmark_unittest)

I was able to fix this via:

diff --git a/libc/benchmarks/CMakeLists.txt b/libc/benchmarks/CMakeLists.txt
index 60f522d7d8c6..8dae3ea94a20 100644
--- a/libc/benchmarks/CMakeLists.txt
+++ b/libc/benchmarks/CMakeLists.txt
@@ -100,7 +100,7 @@ target_include_directories(libc-benchmark
 )
 target_link_libraries(libc-benchmark
     PUBLIC
-    benchmark::benchmark
+    benchmark
     LLVMSupport
     LLVMTargetParser
     Threads::Threads

(Not sure if that's correct?). We can then configure the build, but ninja libc-benchmark-test fails because it can't find the headers. Ok, fixed with:

diff --git a/libc/benchmarks/CMakeLists.txt b/libc/benchmarks/CMakeLists.txt
index 60f522d7d8c6..ccdb00459c8a 100644
--- a/libc/benchmarks/CMakeLists.txt
+++ b/libc/benchmarks/CMakeLists.txt
@@ -96,11 +96,11 @@ add_library(libc-benchmark
 )
 
 target_include_directories(libc-benchmark
-    PUBLIC ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR}
+    PUBLIC ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR} ${CMAKE_BINARY_DIR}/libc/benchmarks/google-benchmark-libc/include/
 )
 target_link_libraries(libc-benchmark
     PUBLIC
-    benchmark::benchmark
+    benchmark
     LLVMSupport
     LLVMTargetParser
     Threads::Threads

and then we fail to find llvm/ADT/ArrayRef.h. It turns out, llvm-project/libc/benchmarks/LibcBenchmark.h depends on llvm/ADT/ArrayRef.h and llvm/ADT/SmallVector.h.

That was surprising. To ensure llvm-libc doesn't have dependencies on LLVM itself, we should move the benchmark code to use our cpp::array and cpp::vector. Until then, I'm just going to remove -DLIBC_INCLUDE_BENCHMARKS=ON and ninja libc-benchmark-test from post submit to unblock the fullbuild builders. cc @gchatelet

@llvmbot
Copy link
Member

llvmbot commented Dec 12, 2024

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

As a result of

llvm/llvm-zorg#325
llvm/llvm-zorg#342

The cmake option -DLIBC_INCLUDE_BENCHMARKS=ON now fails to build when using llvm-projects/runtimes/ as the cmake root.

CMake Error at /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/benchmarks/CMakeLists.txt:35 (add_executable):
  Target "libc-benchmark-test" links to target "benchmark::benchmark" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  /home/libc-buildbot/libc-aarch64-ubuntu/libc-aarch64-ubuntu-fullbuild-dbg/llvm-project/libc/benchmarks/CMakeLists.txt:111 (add_libc_benchmark_unittest)

I was able to fix this via:

diff --git a/libc/benchmarks/CMakeLists.txt b/libc/benchmarks/CMakeLists.txt
index 60f522d7d8c6..8dae3ea94a20 100644
--- a/libc/benchmarks/CMakeLists.txt
+++ b/libc/benchmarks/CMakeLists.txt
@@ -100,7 +100,7 @@ target_include_directories(libc-benchmark
 )
 target_link_libraries(libc-benchmark
     PUBLIC
-    benchmark::benchmark
+    benchmark
     LLVMSupport
     LLVMTargetParser
     Threads::Threads

(Not sure if that's correct?). We can then configure the build, but ninja libc-benchmark-test fails because it can't find the headers. Ok, fixed with:

diff --git a/libc/benchmarks/CMakeLists.txt b/libc/benchmarks/CMakeLists.txt
index 60f522d7d8c6..ccdb00459c8a 100644
--- a/libc/benchmarks/CMakeLists.txt
+++ b/libc/benchmarks/CMakeLists.txt
@@ -96,11 +96,11 @@ add_library(libc-benchmark
 )
 
 target_include_directories(libc-benchmark
-    PUBLIC ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR}
+    PUBLIC ${LLVM_INCLUDE_DIR} ${LLVM_MAIN_INCLUDE_DIR} ${CMAKE_BINARY_DIR}/libc/benchmarks/google-benchmark-libc/include/
 )
 target_link_libraries(libc-benchmark
     PUBLIC
-    benchmark::benchmark
+    benchmark
     LLVMSupport
     LLVMTargetParser
     Threads::Threads

and then we fail to find llvm/ADT/ArrayRef.h. It turns out, llvm-project/libc/benchmarks/LibcBenchmark.h depends on llvm/ADT/ArrayRef.h and llvm/ADT/SmallVector.h.

That was surprising. To ensure llvm-libc doesn't have dependencies on LLVM itself, we should move the benchmark code to use our cpp::array and cpp::vector. Until then, I'm just going to remove -DLIBC_INCLUDE_BENCHMARKS=ON and ninja libc-benchmark-test from post submit to unblock the fullbuild builders. cc @gchatelet

@gchatelet
Copy link
Contributor

Thx for tracking this down @nickdesaulniers. Benchmarks were created at the beginning of the project and we didn't have the CPP folder back then. I agree it should be modernized to avoid the dependency.

@nickdesaulniers
Copy link
Member Author

Getting pretty far down this rabbit hole but it seems that googlemock and googletest in llvm-project/third-party/ are themselves tightly coupled to using llvm/Support/raw_os_ostream.h...arghh

@nickdesaulniers
Copy link
Member Author

@gchatelet do you recall what libc/benchmarks/JSON.cpp is used for?

@gchatelet
Copy link
Contributor

@gchatelet do you recall what libc/benchmarks/JSON.cpp is used for?

It was used by the python script for analysis.
There are basically two benchmarks:

  • The one depending on googlemock and googletest (LibcMemoryGoogleBenchmarkMain), it is convenient and well integrated with other tools and gives a unique number per function / distribution.
  • The one that is entirely custom (LibcMemoryBenchmarkMain) it returns a JSON file that can dig into per size performance.

Another possibility is to drop benchmarking completely and rely on fleetbench's libc suite. Fleetbench has been designed after LibcMemoryGoogleBenchmarkMain. It is actively maintained contrary to this one. The downside is the fact that it will be more difficult to use for someone working in LLVM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants