Skip to content

Let perftools to also bench OpenSSL forks.#79

Open
Sashan wants to merge 1 commit intoopenssl:mainfrom
Sashan:project.1845
Open

Let perftools to also bench OpenSSL forks.#79
Sashan wants to merge 1 commit intoopenssl:mainfrom
Sashan:project.1845

Conversation

@Sashan
Copy link
Contributor

@Sashan Sashan commented Feb 24, 2026

This change adjusts existing perftools so they cne be built by 3rd party SSL libraries (a.k.a. OpenSSL forks). Currently those tools can be used to bench OpenSSL forks:
randbytes, rsasign, handshake, sslnew, x509storeissuer.
evp_setpeer, writeread

The change also adds bench_config_forks.sh which installs forks and builds perftools. And bench_run_forks.sh which runs tests and plots results.

Fixes openssl/project#1845

@Sashan Sashan requested a review from esyr February 24, 2026 15:33
@Sashan Sashan marked this pull request as draft February 24, 2026 15:38
@Sashan Sashan changed the title Let perftools to becnah also OpenSSL forks. Let perftools to also bench OpenSSL forks. Feb 24, 2026
@Sashan Sashan force-pushed the project.1845 branch 2 times, most recently from 817f657 to d02b44e Compare February 24, 2026 17:56
@Sashan Sashan marked this pull request as ready for review February 24, 2026 17:56
@Sashan Sashan force-pushed the project.1845 branch 2 times, most recently from d09e327 to b47c395 Compare February 24, 2026 21:54
@Sashan
Copy link
Contributor Author

Sashan commented Feb 24, 2026

If you want to test thsoe changes to play around with them you need to apply diff below to cloned P$:

diff --git a/bench-scripts/bench_config_forks.sh b/bench-scripts/bench_config_forks.sh
index 9d21b52..96c2799 100755
--- a/bench-scripts/bench_config_forks.sh
+++ b/bench-scripts/bench_config_forks.sh
@@ -28,14 +28,14 @@ function build_perftools {
     # you must change link to repository, so script pulls
     # modified sources
     #
-    typeset PERFTOOLS='https://github.com/openssl/perftools'
+    typeset PERFTOOLS='https://github.com/sashan/perftools'
 
     cd ${WORKSPACE_ROOT} || exit 1
     #
     # you may also need to change clone command to
     # checkout correct branch.
     #
-    git clone ${PERFTOOLS} || exit 1
+    git clone -b project.1845 ${PERFTOOLS} || exit 1
     cd perftools/source || exit 1
 
     cmake -S ${WORKSPACE_ROOT}/perftools/source \

@@ -0,0 +1,88 @@
#!/usr/bin/env ksh
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason we're using ksh here as opposed to just sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got used to typeset to limit the scope of variables in shell. and I think typeset is ksh specific. I don't know if there is portable equivalent of typeset.

cd "${BORING_NAME}"
git clone "${BORING_REPO}" --depth 1 . || exit 1
git clone "${BORING_REPO}" . || exit 1
git checkout 0.20251124.0 || exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to run with a pinned version of boring here? Or should be be using the latest on master, since we're (I think) trying to get regular performance numbers updated over time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the master branch may not build. it happened to me once or twice. hence I opted for a tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the code to use master branch and added comment how to work around in case of breakgae.

@Sashan Sashan marked this pull request as draft March 6, 2026 09:18
@Sashan
Copy link
Contributor Author

Sashan commented Mar 6, 2026

The code currently depends on .ksh script which does not fit well with ansible target after poking to cmake documentation I think it's worth to try bit harder and use cmake to fetch and build 3rd-party openssl libraries.

the PR is going to land after #86 anyway. and #86 creates enough conflicts to resolve.

@Sashan Sashan force-pushed the project.1845 branch 2 times, most recently from bd4845a to 38f27a9 Compare March 8, 2026 22:42
@Sashan Sashan requested a review from nhorman March 8, 2026 22:43
@Sashan
Copy link
Contributor Author

Sashan commented Mar 8, 2026

needs review the CMakeLists.txt after rebase.

@Sashan
Copy link
Contributor Author

Sashan commented Mar 9, 2026

close + re-open to re-run CI.

the failing tests correctly find openssl-4.0 but they fail to find SSL_set1_dnsname() symbol.

@Sashan Sashan closed this Mar 9, 2026
@Sashan Sashan reopened this Mar 9, 2026
@Sashan Sashan force-pushed the project.1845 branch 2 times, most recently from 056d574 to 2404277 Compare March 9, 2026 12:14
@Sashan
Copy link
Contributor Author

Sashan commented Mar 9, 2026

just for the record, this is the change which makes CI happy

diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
index 56bd224..8a1d186 100644
--- a/source/CMakeLists.txt
+++ b/source/CMakeLists.txt
@@ -205,7 +205,6 @@ else()
     find_package(OpenSSL REQUIRED ${OPENSSL_CONFIG_MODE})
     target_link_libraries(perf PUBLIC OpenSSL::SSL OpenSSL::Crypto)
     set(CMAKE_REQUIRED_LIBRARIES OpenSSL::SSL OpenSSL::Crypto)
-    set(CMAKE_REQUIRED_LIBRARIES OpenSSL::Crypto)
     set(CMAKE_REQUIRED_INCLUDES "${OPENSSL_INCLUDE_DIR}")
 endif()

the offending line got introduced during conflict resolution when rebasing to main branch

my local test systems use cmake version 3.31, the github instance seems to be using 3.16. it's interesting the local systems with cmake3.31 don't notice the issue everything works there.

I was lucky to meet my cmake --trace-expand friend which produces very verbose log on github. this item in log file helped me to figure out what went wrong:

     1	2026-03-09T12:39:28.0714710Z Linking C executable cmTC_cd7ad
     2	2026-03-09T12:39:28.0715130Z /usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_cd7ad.dir/link.txt --verbose=1
     3	2026-03-09T12:39:28.0716560Z /usr/bin/cc -DDEBUG -Werror -Wall     CMakeFiles/cmTC_cd7ad.dir/CheckSymbolExists.c.o  -o cmTC_cd7ad  -Wl,-rpath,/__w/perftools/perftools/openssl /__w/perftools/perftools/openssl/libcrypto.so 
     4	2026-03-09T12:39:28.0717014Z /usr/bin/ld: CMakeFiles/cmTC_cd7ad.dir/CheckSymbolExists.c.o: in function `main':
     5	2026-03-09T12:39:28.0717423Z CheckSymbolExists.c:(.text+0x1f): undefined reference to `SSL_set1_dnsname'
     6	2026-03-09T12:39:28.0717611Z collect2: error: ld returned 1 exit status

line 3 shows the linker command to build a binary. note the linker uses libcrypto.so instead of libssl.so. This brought my attention to set(CMAKE_REQUIRED_LIBRARIES, ...) line. Have not investigated further if the change in behavior is caused by cmake version (3.16 vs. 3.31) or by another difference in environements.

This change adjusts existing perftools so they cne be built
by 3rd party SSL libraries (a.k.a. OpenSSL forks). Currently
those tools can be used to bench OpenSSL forks:
   randbytes, rsasign, handshake, sslnew, x509storeissuer.
   evp_setpeer, writeread

The change also adds bench_config_forks.sh which installs
forks and builds perftools. And bench_run_forks.sh which
runs tests and plots results.

Fixes openssl/project#1845
@Sashan Sashan marked this pull request as ready for review March 9, 2026 13:23
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.

make perftools OpenSSL agnostic so we can use them with openssl forks

2 participants