Skip to content
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

Add benchmark_main.pc to link main() containing library #1779

Merged
merged 2 commits into from
Apr 14, 2024

Conversation

SoapGentoo
Copy link
Contributor

This is similar to the addition in google/googletest@8604c4a#diff-eb8e49bdf5e9aafb996777a4f4302ad1efd281222bf3202eb9b77ce47496c345 that added pkg-config support in GTest. Without this, users need to manually find the library containing main().

This is similar to the addition in google/googletest@8604c4a#diff-eb8e49bdf5e9aafb996777a4f4302ad1efd281222bf3202eb9b77ce47496c345
that added pkg-config support in GTest. Without this, users
need to manually find the library containing `main()`.
@LebedevRI
Copy link
Collaborator

Just out of abundance of caution, could you please show that a simple benchmark can be compiled and linked and run using that .pc?

@LebedevRI
Copy link
Collaborator

My main point of contention is,

target_link_libraries(benchmark_main PUBLIC benchmark::benchmark)

does it actually happen to transitively link to the benchmark, like the CMake-provided one does?

@SoapGentoo
Copy link
Contributor Author

Yes it does, since it Requires: benchmark which means it always links to libbenchmark. I will prepare a sample invocation for you

@SoapGentoo
Copy link
Contributor Author

Code:

#include <benchmark/benchmark.h>

#include <chrono>
#include <thread>

static void BM_SomeFunction(benchmark::State& state) {
  for (auto _ : state) {
    std::this_thread::sleep_for(std::chrono::milliseconds(1000));
  }
}

// Register the function as a benchmark
BENCHMARK(BM_SomeFunction);

Running (just relying on benchmark.pc)

$ set -x; g++ -O2 $(pkg-config --cflags benchmark) $(pkg-config --libs benchmark) test_pkgconfig.cpp && ./a.out
++ pkg-config --cflags benchmark
++ pkg-config --libs benchmark
+ g++ -O2 -I/tmp/PREFIX/include -L/tmp/PREFIX/lib64 -lbenchmark test_pkgconfig.cpp
/usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: /tmp/ccw31bMJ.o: warning: relocation against `_ZTVN9benchmark8internal17FunctionBenchmarkE' in read-only section `.text.startup'
/usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../lib64/Scrt1.o: in function `_start':
(.text+0x1b): undefined reference to `main'

with benchmark_main.pc:

$ set -x; g++ -O2 $(pkg-config --cflags benchmark_main) $(pkg-config --libs benchmark_main) test_pkgconfig.cpp && ./a.out
+ set -x
++ pkg-config --cflags benchmark_main
++ pkg-config --libs benchmark_main
+ g++ -O2 -I/tmp/PREFIX/include -L/tmp/PREFIX/lib64 -lbenchmark_main -lbenchmark test_pkgconfig.cpp
+ ./a.out
2024-04-14T17:47:36+02:00
Running ./a.out
Run on (32 X 5083.4 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x16)
  L1 Instruction 32 KiB (x16)
  L2 Unified 512 KiB (x16)
  L3 Unified 32768 KiB (x2)
Load Average: 0.43, 0.38, 0.22
***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead.
***WARNING*** Library was built as DEBUG. Timings may be affected.
----------------------------------------------------------
Benchmark                Time             CPU   Iterations
----------------------------------------------------------
BM_SomeFunction 1000136861 ns         9060 ns           10

Copy link
Collaborator

@LebedevRI LebedevRI left a comment

Choose a reason for hiding this comment

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

LG, thank you!

@LebedevRI LebedevRI merged commit c010560 into google:main Apr 14, 2024
80 checks passed
@SoapGentoo SoapGentoo deleted the benchmark_main.pc branch April 14, 2024 16:54
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.

None yet

2 participants