Fixes #1031: Added new option for PGO called ENABLE_PROFILE_GUIDED_OP… (

#1044) * Fixes #1031: Added new option for PGO called ENABLE_PROFILE_GUIDED_OPTIMIZATION. * Code review changes: Set QPID_SYSTEM_TEST_TIMEOUT=300 before executing tests
skupperproject · Jun 9, 2023 · fc134fb · fc134fb
1 parent 8fdaaf2
commit fc134fb
Show file tree

Hide file tree

Showing 6 changed files with 199 additions and 19 deletions.
diff --git a/.github/scripts/compile.sh b/.github/scripts/compile.sh
@@ -130,6 +130,13 @@ do_build () {
   local suffix=${1}
   local runtime_check=${2}
 
+  if [ "$runtime_check" == "OFF" ]; then
+     # Turn on PGO only when we are not doing asan or tsan
+     BUILD_OPTS="-DENABLE_PROFILE_GUIDED_OPTIMIZATION=ON"
+  else
+     BUILD_OPTS="-DBUILD_TESTING=OFF"
+  fi
+
   cmake -S "${PROTON_DIR}" -B "${PROTON_BUILD_DIR}${suffix}" \
     -DCMAKE_BUILD_TYPE=RelWithDebInfo \
     -DRUNTIME_CHECK="${runtime_check}" \
@@ -142,21 +149,31 @@ do_build () {
 
   DESTDIR="$PROTON_INSTALL_DIR${suffix}" cmake --install "$PROTON_BUILD_DIR${suffix}"
 
+   if [ "$runtime_check" == "OFF" ]; then
+     # This will install the proton python libraries in sys.path so the tests using
+     # proton can be run successfully.
+     python3 -m pip install "$(find "$PROTON_BUILD_DIR/python/" -name 'python-qpid-proton-*.tar.gz')"
+   fi
+
   cmake -S "${SKUPPER_DIR}" -B "${SKUPPER_BUILD_DIR}${suffix}" \
     -DCMAKE_BUILD_TYPE=RelWithDebInfo \
     -DRUNTIME_CHECK="${runtime_check}" \
     -DProton_USE_STATIC_LIBS=ON \
     -DProton_DIR="${PROTON_INSTALL_DIR}${suffix}/usr/lib64/cmake/Proton" \
-    -DBUILD_TESTING=OFF \
+    ${BUILD_OPTS} \
     -DVERSION="${VERSION}" \
     -DCMAKE_INSTALL_PREFIX=/usr
   cmake --build "${SKUPPER_BUILD_DIR}${suffix}" --verbose
 }
 
+# This is required to install the python packages that the system tests use.
+python3 -m pip install -r "${SKUPPER_DIR}"/requirements-dev.txt
+
 # Do a regular build without asan or tsan.
 do_build "" OFF
-# and install Proton Python
-python3 -m pip install --prefix="$PROTON_INSTALL_DIR/usr" "$(find "$PROTON_BUILD_DIR/python/" -name 'python-qpid-proton-*.tar.gz')"
+
+# Install Proton Python
+python3 -m pip install --ignore-installed --prefix="$PROTON_INSTALL_DIR/usr" "$(find "$PROTON_BUILD_DIR/python/" -name 'python-qpid-proton-*.tar.gz')"
 
 # Then perform sanitized builds of Proton and the Router.
 # talking to annobin is not straightforward, https://bugzilla.redhat.com/show_bug.cgi?id=1536569

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
@@ -794,6 +794,115 @@ jobs:
         working-directory: ${{env.DispatchBuildDir}}
         run: ctest -VV -R python-checker
 
+  pgo:
+    name: Profile Guided Optimization
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-20.04]
+        container: ['fedora']
+        containerTag: ['37']
+        buildType: [RelWithDebInfo]
+        protonGitRef:
+          - ${{ github.event.inputs.protonBranch || 'main' }}
+    container:
+      image: 'quay.io/${{ matrix.container }}/${{ matrix.container }}:${{ matrix.containerTag }}'
+      volumes:
+        - ${{github.workspace}}:${{github.workspace}}
+      options: --privileged --ulimit core=-1 --security-opt apparmor:unconfined --security-opt seccomp=unconfined --sysctl net.ipv6.conf.all.disable_ipv6=0
+    env:
+      BuildType: ${{matrix.buildType}}
+      ProtonBuildDir: ${{github.workspace}}/qpid-proton/build
+      RouterBuildDir: ${{github.workspace}}/skupper-router/build
+      InstallPrefix: ${{github.workspace}}/install
+      # this enables colored output
+      FORCE_COLOR: 1
+      COLUMNS: 160
+      ProtonCMakeExtraArgs: >
+        -DCMAKE_C_COMPILER_LAUNCHER=ccache
+        -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
+        -DBUILD_BINDINGS=python
+        -DPython_EXECUTABLE=/usr/bin/python3
+        -DBUILD_EXAMPLES=OFF
+        -DBUILD_TESTING=OFF
+        -DENABLE_FUZZ_TESTING=OFF
+        -DRUNTIME_CHECK=OFF
+        -DBUILD_TLS=ON
+      RouterCMakeExtraArgs: >
+        -DCMAKE_C_COMPILER_LAUNCHER=ccache
+        -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
+        -DPython_EXECUTABLE=/usr/bin/python3
+        -DRUNTIME_CHECK=OFF
+        -DENABLE_PROFILE_GUIDED_OPTIMIZATION=ON
+      CCACHE_BASEDIR: ${{github.workspace}}
+      CCACHE_DIR: ${{github.workspace}}/.ccache
+      CCACHE_COMPRESS: 'true'
+      CCACHE_MAXSIZE: '400MB'
+      QPID_SYSTEM_TEST_TIMEOUT: 400
+      VERBOSE: 1
+
+    steps:
+      - name: Show environment (Linux)
+        if: ${{ always() && runner.os == 'Linux' }}
+        run: env -0 | sort -z | tr '\0' '\n'
+
+      - uses: actions/checkout@v3
+        with:
+          repository: ${{ env.protonRepository }}
+          ref: ${{ matrix.protonGitRef }}
+          path: 'qpid-proton'
+
+      - uses: actions/checkout@v3
+        with:
+          path: 'skupper-router'
+
+      - name: Install Linux build dependencies
+        run: |
+          dnf install -y gcc gcc-c++ elfutils-devel cmake libuuid-devel openssl openssl-devel cyrus-sasl-devel cyrus-sasl-plain swig make libwebsockets-devel libnghttp2-devel ccache
+
+      - name: Install Linux build dependencies (Python)
+        run: dnf install -y python3-devel python3-pip
+
+      - name: Install Linux runtime/test dependencies
+        run: |
+          dnf install -y curl nmap-ncat gdb binutils elfutils elfutils-debuginfod-client findutils file nginx
+          dnf debuginfo-install -y python3
+
+      - name: Install Python build dependencies
+        run: python3 -m pip install setuptools wheel tox
+
+      - name: Install Python runtime/test dependencies
+        run: python3 -m pip install -r ${{github.workspace}}/skupper-router/requirements-dev.txt
+
+      - name: Create Build and Install directories
+        run: mkdir -p "${ProtonBuildDir}" "${RouterBuildDir}" "${InstallPrefix}"
+
+      - name: qpid-proton cmake configure
+        working-directory: ${{env.ProtonBuildDir}}
+        run: >
+          cmake "${{github.workspace}}/qpid-proton" \
+            "-DCMAKE_INSTALL_PREFIX=${InstallPrefix}" \
+            "-DCMAKE_BUILD_TYPE=${BuildType}" \
+            ${ProtonCMakeExtraArgs}
+      - name: qpid-proton cmake build/install
+        run: cmake --build "${ProtonBuildDir}" --config ${BuildType} --target install --parallel 6
+
+      - name: install qpid-proton python wheel
+        run: python3 -m pip install $(find ${ProtonBuildDir}/python/ -name 'python_qpid_proton*.whl')
+
+      - name: skupper-router cmake configure
+        working-directory: ${{env.RouterBuildDir}}
+        run: >
+          cmake "${{github.workspace}}/skupper-router" \
+            "-DCMAKE_INSTALL_PREFIX=${InstallPrefix}" \
+            "-DCMAKE_BUILD_TYPE=${BuildType}" \
+            "-DPYTHON_TEST_COMMAND='-m;pytest;-vs;--timeout=400;--junit-prefix=pytest.\${py_test_module};--junit-xml=junitxmls/\${py_test_module}.xml;--pyargs;\${py_test_module}'" \
+            ${RouterCMakeExtraArgs} -DSANITIZE_PYTHON=${RouterCMakeSanitizePython:-ON}
+
+      - name: skupper-router cmake build/install
+        run: cmake --build "${RouterBuildDir}" --config ${BuildType} --target install --parallel 6
+
   container:
     name: Container image
     runs-on: ubuntu-latest

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -34,6 +34,7 @@ set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
 set(CMAKE_ENABLE_EXPORTS TRUE)
 option(CMAKE_INTERPROCEDURAL_OPTIMIZATION "Perform link time optimization" ON)
 option(ENABLE_WARNING_ERROR "Consider compiler warnings to be errors" ON)
+option(ENABLE_PROFILE_GUIDED_OPTIMIZATION "Perform profile guided optimization" OFF)
 
 # preserve frame pointers for ease of debugging and profiling
 #  see https://fedoraproject.org/wiki/Changes/fno-omit-frame-pointer
@@ -47,6 +48,18 @@ set(C_WARNING_Clang -Wall -Wextra -Wpedantic
         -Wstrict-prototypes -Wold-style-definition
         -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare -Wno-language-extension-token)
 
+# Set default build type. Must use FORCE because project() sets default to ""
+if (NOT CMAKE_BUILD_TYPE)
+  set (CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING
+    "Build type: Debug, Release, RelWithDebInfo, MinSizeRel or Coverage (default RelWithDebInfo)" FORCE)
+endif(NOT CMAKE_BUILD_TYPE)
+if (CMAKE_BUILD_TYPE MATCHES "Deb|Cover")
+  set (has_debug_symbols " (has debug symbols)")
+endif (CMAKE_BUILD_TYPE MATCHES "Deb|Cover")
+message(STATUS "Build type is \"${CMAKE_BUILD_TYPE}\"${has_debug_symbols}")
+# This is commonly needed so define it before we include anything else.
+string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
+
 if (CMAKE_INTERPROCEDURAL_OPTIMIZATION)
     include(CheckIPOSupported)
     check_ipo_supported(RESULT ipo_supported OUTPUT output)
@@ -66,6 +79,26 @@ endif (CMAKE_INTERPROCEDURAL_OPTIMIZATION)
 set(C_EXTRA_GNU )
 set(C_EXTRA_Clang )
 
+if (ENABLE_PROFILE_GUIDED_OPTIMIZATION)
+    message(STATUS "Profile guided optimization is enabled on ${CMAKE_C_COMPILER_ID} compiler")
+    # Set the build type to coverage since we need coverage related
+    # build options when -fprofile-generate is used.
+    if("GNU" STREQUAL ${CMAKE_C_COMPILER_ID})
+        # Temporarily modify the CMAKE_BUILD_TYPE to Coverage. Note that we are not setting the
+        # CMAKE_BUILD_TYPE to Coverage using the CACHE option. This means that when this build is over,
+        # we will retain the original value of CMAKE_BUILD_TYPE
+        set(CMAKE_BUILD_TYPE Coverage)
+        #  You must use -fprofile-generate both when compiling *and* when linking your program
+        set(C_EXTRA_GNU ${C_EXTRA_GNU} -fprofile-generate=${CMAKE_BINARY_DIR}/profile-data -fprofile-correction)
+        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-generate=${CMAKE_BINARY_DIR}/profile-data")
+    endif("GNU" STREQUAL ${CMAKE_C_COMPILER_ID})
+    if("Clang" STREQUAL ${CMAKE_C_COMPILER_ID})
+        # We are currently not supporting PGO on Clang although clang has good options to do PGO.
+        # Any contributions to skupper-router in this regard is welcome.
+        message(FATAL_ERROR "Profile Guided optimization of skupper-router not supported on ${CMAKE_C_COMPILER_ID}. Please consider contributing this feature")
+    endif("Clang" STREQUAL ${CMAKE_C_COMPILER_ID})
+endif (ENABLE_PROFILE_GUIDED_OPTIMIZATION)
+
 set(C_WARNING_FLAGS ${C_WARNING_${CMAKE_C_COMPILER_ID}})
 set(C_EXTRA_FLAGS ${C_EXTRA_${CMAKE_C_COMPILER_ID}})
 add_compile_options(${C_WARNING_FLAGS} ${C_EXTRA_FLAGS})
@@ -74,18 +107,6 @@ if (ENABLE_WARNING_ERROR)
     add_compile_options(-Werror)
 endif (ENABLE_WARNING_ERROR)
 
-# Set default build type. Must use FORCE because project() sets default to ""
-if (NOT CMAKE_BUILD_TYPE)
-  set (CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING
-    "Build type: Debug, Release, RelWithDebInfo, MinSizeRel or Coverage (default RelWithDebInfo)" FORCE)
-endif(NOT CMAKE_BUILD_TYPE)
-if (CMAKE_BUILD_TYPE MATCHES "Deb|Cover")
-  set (has_debug_symbols " (has debug symbols)")
-endif (CMAKE_BUILD_TYPE MATCHES "Deb|Cover")
-message(STATUS "Build type is \"${CMAKE_BUILD_TYPE}\"${has_debug_symbols}")
-# This is commonly needed so define it before we include anything else.
-string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
-
 if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
     option(QD_ENABLE_ASSERTIONS "Enable assertions" ON)
 else()
@@ -257,11 +278,17 @@ endif()
 
 # Set up extra coverage analysis options for gcc and clang
 if (CMAKE_BUILD_TYPE MATCHES "Coverage")
- set (CMAKE_C_FLAGS_COVERAGE "-g -O0 --coverage")
- set (CMAKE_CXX_FLAGS_COVERAGE "-g -O0 --coverage")
+ if (ENABLE_PROFILE_GUIDED_OPTIMIZATION)
+   set (CMAKE_C_FLAGS_COVERAGE "-g -O2 -DNDEBUG --coverage")
+   set (CMAKE_CXX_FLAGS_COVERAGE "-g -O2  --coverage")
+ else(ENABLE_PROFILE_GUIDED_OPTIMIZATION)
+   set (CMAKE_C_FLAGS_COVERAGE "-g -O0 --coverage")
+   set (CMAKE_CXX_FLAGS_COVERAGE "-g -O0 --coverage")
+ endif(ENABLE_PROFILE_GUIDED_OPTIMIZATION)
  set (CMAKE_EXE_LINKER_FLAGS_COVERAGE "--coverage")
  set (CMAKE_MODULE_LINKER_FLAGS_COVERAGE "--coverage")
  set (CMAKE_SHARED_LINKER_FLAGS_COVERAGE "--coverage")
+
  mark_as_advanced(
    CMAKE_C_FLAGS_COVERAGE
    CMAKE_EXE_LINKER_FLAGS_COVERAGE

diff --git a/router/CMakeLists.txt b/router/CMakeLists.txt
@@ -37,4 +37,30 @@ add_executable(skrouterd ${router_SOURCES})
 target_link_libraries(skrouterd skupper-router)
 target_link_options(skrouterd PUBLIC LINKER:-Map=skrouterd.map)
 
+if (ENABLE_PROFILE_GUIDED_OPTIMIZATION)
+    if("GNU" STREQUAL ${CMAKE_C_COMPILER_ID})
+        add_custom_command(TARGET skrouterd
+            POST_BUILD
+            # Run the tests with the profiled skrouterd binary
+            # Running the tests produce the files with the .gcda extension in the build/profile-dir folder
+            # For now, we are only running the adaptor based tests to keep it quick.
+            # Even if one of the adaptor tests fail, we will go ahead and create the final executable.
+            COMMAND QPID_SYSTEM_TEST_TIMEOUT=300 ${CMAKE_CTEST_COMMAND} -j4 -V -R tcp || (exit 0)
+            COMMAND QPID_SYSTEM_TEST_TIMEOUT=300 ${CMAKE_CTEST_COMMAND} -j4 -V -R http || (exit 0)
+            # Set the ENABLE_PROFILE_GUIDED_OPTIMIZATION=OFF so that we don't once again create profile data.
+            COMMAND ${CMAKE_COMMAND} ${CMAKE_BINARY_DIR} -DENABLE_PROFILE_GUIDED_OPTIMIZATION=OFF -DCMAKE_EXE_LINKER_FLAGS=-fprofile-use=${CMAKE_BINARY_DIR}/profile-data
+
+            # Feed the profile data we created from the above test runs using the -fprofile-use compiler option.
+            # Use the -fprofile-correction option to smoothen the counter updates when using multi-threaded applications.
+            COMMAND ${CMAKE_COMMAND} ${CMAKE_BINARY_DIR} -DC_EXTRA_GNU=-fprofile-use=${CMAKE_BINARY_DIR}/profile-data\\\;-fprofile-correction\\\;-fprofile-partial-training
+            COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target skrouterd
+
+            # Set back the ENABLE_PROFILE_GUIDED_OPTIMIZATION=ON, so can leave it same as when we started the build.
+            COMMAND ${CMAKE_COMMAND} ${CMAKE_BINARY_DIR} -DENABLE_PROFILE_GUIDED_OPTIMIZATION=ON
+
+            WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
+        )
+    endif("GNU" STREQUAL ${CMAKE_C_COMPILER_ID})
+endif (ENABLE_PROFILE_GUIDED_OPTIMIZATION)
+
 install(TARGETS skrouterd RUNTIME DESTINATION sbin)
diff --git a/tests/system_test.py b/tests/system_test.py
@@ -1149,7 +1149,7 @@ def rmtree(self):
     def setup(self):
         """Called from test setup and class setup."""
         if self.directory:
-            os.makedirs(self.directory)
+            os.makedirs(self.directory, exist_ok=True)
             os.chdir(self.directory)
 
     def _next_port(self) -> int:

diff --git a/tests/system_tests_tcp_adaptor_tls.py b/tests/system_tests_tcp_adaptor_tls.py
@@ -449,7 +449,8 @@ def test_connector_requires_client_auth_fail(self):
         # going to be happy.
         _, _ = self.opensslclient(port=self.router_listener_port_server_auth_peer,
                                   ssl_info=ssl_info,
-                                  data=b"test_connector_requires_client_auth")
+                                  data=b"test_connector_requires_client_auth",
+                                  expect=Process.EXIT_OK)
         # There will be an message on the openssl server out file that the peer did not return a certificate since
         # the corresponding connector sslProfile did not have a client cert set on it.
         self.openssl_server_auth_peer.wait_out_message("SSL routines:tls_process_client_certificate:peer "