From 759d70deee2c2f0b9b546121ffd9d7a4df57cb79 Mon Sep 17 00:00:00 2001 From: Eliran Sinvani Date: Thu, 28 Dec 2023 06:32:01 +0200 Subject: [PATCH] configure.py support coverage profiles on standrad build modes We already have a dedicated coverage build, however, this build is dedicated mostly for coverage in boost and standalone unit tests. This added configuration option will compile every configured build mode with coverage profiling support (excluding 'coverage' mode). It also does targeted profiling that is narrowed down only to ScyllaDB code and doesn't instrument seastar and testing code, this should give a more accurate coverage reporting and also impact performance less, as one example, the reactor loop in seastar will not be profiled (along with everything else). The targeted profiling is done with the help of the newly added `coverage_sources.list` file which excludes all seastar sub directories from the profiling. Also an extra measure is taken to make sure that the seastar library will not be linked with the coverage framework (so it will not dump confusing empty profiles). Some of the seastar headers are still going to be included in the profile since they are indirectly included by profiled source files in order to remove them from the final report a processing step on the resulting profile will need to take place. A note about expected performance impact: It is expected to have minimal impact on performance since the instrumentation adds counter increments without locking. Ref: https://clang.llvm.org/docs/UsersManual.html#cmdoption-fprofile-update This means that the numbers themselves are less reliable but all covered lines are guarantied to have at least non-zero value. Signed-off-by: Eliran Sinvani --- configure.py | 20 +++++++++++++++++++- coverage_sources.list | 8 ++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 coverage_sources.list diff --git a/configure.py b/configure.py index 3162b61854..79aefe6ac2 100755 --- a/configure.py +++ b/configure.py @@ -778,8 +778,11 @@ arg_parser.add_argument('--list-artifacts', dest='list_artifacts', action='store arg_parser.add_argument('--date-stamp', dest='date_stamp', type=str, help='Set datestamp for SCYLLA-VERSION-GEN') arg_parser.add_argument('--use-cmake', action='store_true', help='Use CMake as the build system') +arg_parser.add_argument('--coverage', action = 'store_true', help = 'Compile scylla with coverage instrumentation') args = arg_parser.parse_args() +PROFILES_LIST_FILE_NAME = "coverage_sources.list" + if args.list_artifacts: for artifact in sorted(all_artifacts): print(artifact) @@ -1418,6 +1421,15 @@ tests_not_using_seastar_test_framework = set([ 'test/unit/cross_shard_barrier_test', ]) | pure_boost_tests + +COVERAGE_INST_FLAGS = ['-fprofile-instr-generate', '-fcoverage-mapping', f'-fprofile-list=./{PROFILES_LIST_FILE_NAME}'] +if args.coverage: + for _, mode in filter(lambda m: m[0] != "coverage", modes.items()): + mode['cxx_ld_flags'] += ' ' + ' '.join(COVERAGE_INST_FLAGS) + mode['cxx_ld_flags'] = mode['cxx_ld_flags'].strip() + mode['cxxflags'] += ' ' + ' '.join(COVERAGE_INST_FLAGS) + mode['cxxflags'] = mode['cxxflags'].strip() + for t in tests_not_using_seastar_test_framework: if t not in scylla_tests: raise Exception("Test %s not found in scylla_tests" % (t)) @@ -1671,13 +1683,19 @@ def real_relpath(path, start): def configure_seastar(build_dir, mode, mode_config): seastar_build_dir = os.path.join(build_dir, mode, 'seastar') + seastar_cxx_ld_flags = mode_config['cxx_ld_flags'] + # We want to "undo" coverage for seastar if we have it enabled. + if args.coverage: + for flag in COVERAGE_INST_FLAGS: + seastar_cxx_ld_flags = seastar_cxx_ld_flags.replace(' ' + flag, '') + seastar_cxx_ld_flags = seastar_cxx_ld_flags.replace(flag, '') seastar_cmake_args = [ '-DCMAKE_BUILD_TYPE={}'.format(mode_config['cmake_build_type']), '-DCMAKE_C_COMPILER={}'.format(args.cc), '-DCMAKE_CXX_COMPILER={}'.format(args.cxx), '-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON', '-DSeastar_CXX_FLAGS=SHELL:{}'.format(mode_config['lib_cflags']), - '-DSeastar_LD_FLAGS={}'.format(semicolon_separated(mode_config['lib_ldflags'], mode_config['cxx_ld_flags'])), + '-DSeastar_LD_FLAGS={}'.format(semicolon_separated(mode_config['lib_ldflags'], seastar_cxx_ld_flags)), '-DSeastar_CXX_DIALECT=gnu++20', '-DSeastar_API_LEVEL=7', '-DSeastar_UNUSED_RESULT_ERROR=ON', diff --git a/coverage_sources.list b/coverage_sources.list new file mode 100644 index 0000000000..d87997c6a8 --- /dev/null +++ b/coverage_sources.list @@ -0,0 +1,8 @@ +# Those tests are testing header files so we have no choice but to instrument them +source:test/boost/small_vector_test\.cc=allow +source:test/boost/anchorless_list_test\.cc=allow + +# Don't instrument files that are part of the testing framework itself. +source:test/*\.*=skip + +default:allow