From 039be30750f8b6b15439c9eecc4f6c63c9aa870e Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Wed, 13 Jul 2022 11:50:54 -0400 Subject: [PATCH] Fix benchmark tests that are failing on Ubuntu and MacOS (#5656) * Updated libuv inclusion in cmake files for object store and benchmarks tests * Updated benchmark-crud to use test_path * Added help to common-tasks and crud benchmark tests * Added 'enable_stdfilesystem' to allow use of std::filesystem in the benchmark tests * Added change description and PR to CHANGELOG * Updated cross-compile.sh to build Android on MacOS * Updated build to (hopefully) not build the libuv.so and work on both Linux and Android * Update tools/cmake/android.toolchain.cmake per review Co-authored-by: Yavor Georgiev --- CHANGELOG.md | 3 +- evergreen/config.yml | 2 +- test/benchmark-common-tasks/main.cpp | 22 +++++++++++-- test/benchmark-crud/main.cpp | 34 +++++++++++++++++++-- test/object-store/CMakeLists.txt | 1 + test/object-store/benchmarks/CMakeLists.txt | 8 +++++ test/util/CMakeLists.txt | 6 ++-- tools/cmake/android.toolchain.cmake | 16 ++++++++++ tools/cross_compile.sh | 14 ++++++--- 9 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 tools/cmake/android.toolchain.cmake diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d15cea196..589e5ec9f2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,8 @@ ### Internals * Add support for running the object store tests and some of the benchmarks on iOS ([PR #5577](https://github.com/realm/realm-core/pull/5577)). * MemRef is now trivially copyable. -* Fix bloaty CI test that is currently failing on master. ([PR #5650](https://github.com/realm/realm-core/pull/5650)) +* Fix bloaty CI test that is currently failing on Ubuntu. ([PR #5650](https://github.com/realm/realm-core/pull/5650)) +* Fix benchmark tests that are failing on Ubuntu and MacOS. ([PR #5656](https://github.com/realm/realm-core/pull/5656)) ---------------------------------------------- diff --git a/evergreen/config.yml b/evergreen/config.yml index 2505d736082..fb1eb0691a8 100644 --- a/evergreen/config.yml +++ b/evergreen/config.yml @@ -153,7 +153,7 @@ functions: mkdir benchmark_results cd benchmark_results - $BENCHMARK + $BENCHMARK "$(pwd)/" - command: perf.send params: file: './realm-core/benchmark_results/results.latest.json' diff --git a/test/benchmark-common-tasks/main.cpp b/test/benchmark-common-tasks/main.cpp index a57928624f6..1ce031bf9ef 100644 --- a/test/benchmark-common-tasks/main.cpp +++ b/test/benchmark-common-tasks/main.cpp @@ -2024,7 +2024,7 @@ void run_benchmark(BenchmarkResults& results, bool force_full = false) // Open a SharedGroup: realm::test_util::DBTestPathGuard realm_path( - test_util::get_test_path("benchmark_common_tasks" + ident, ".realm")); + test_util::get_test_path("benchmark_common_tasks_" + ident, ".realm")); DBRef group; group = create_new_shared_group(realm_path, level, key); benchmark.before_all(group); @@ -2069,7 +2069,9 @@ extern "C" int benchmark_common_tasks_main(); int benchmark_common_tasks_main() { - std::string results_file_stem = realm::test_util::get_test_path_prefix() + "results"; + std::string results_file_stem = realm::test_util::get_test_path_prefix(); + std::cout << "Results path: " << results_file_stem << std::endl; + results_file_stem += "results"; BenchmarkResults results(40, "benchmark-common-tasks", results_file_stem.c_str()); #define BENCH(B) run_benchmark(results) @@ -2151,6 +2153,22 @@ int benchmark_common_tasks_main() int main(int argc, const char** argv) { + if (argc > 1) { + std::string arg_path = argv[1]; + if (arg_path == "-h" || arg_path == "--help") { + std::cout << "Usage: " << argv[0] << " [-h|--help] [PATH]" << std::endl + << "Run the common tasks benchmark test application." << std::endl + << "Results are placed in the executable directory by default." << std::endl + << std::endl + << "Arguments:" << std::endl + << " -h, --help display this help" << std::endl + << " PATH alternate path to store the results files;" << std::endl + << " this path should end with a slash." << std::endl + << std::endl; + return 1; + } + } + if (!initialize_test_path(argc, argv)) return 1; return benchmark_common_tasks_main(); diff --git a/test/benchmark-crud/main.cpp b/test/benchmark-crud/main.cpp index f4b459be657..2c864821959 100644 --- a/test/benchmark-crud/main.cpp +++ b/test/benchmark-crud/main.cpp @@ -26,6 +26,7 @@ #include "../util/timer.hpp" #include "../util/random.hpp" #include "../util/benchmark_results.hpp" +#include "../util/test_path.hpp" using namespace realm; using namespace realm::util; @@ -98,8 +99,9 @@ inline void erase(TableRef table, const OrderVec& order) } // anonymous namepsace +extern "C" int benchmark_crud_main(); -int main() +int benchmark_crud_main() { const size_t target_size = 1100 * 100L; const int num_tables = 20; @@ -148,7 +150,10 @@ int main() int_fast64_t dummy = 0; int max_lead_text_size = 26; - BenchmarkResults results(max_lead_text_size, "benchmark-crud"); + std::string results_file_stem = realm::test_util::get_test_path_prefix(); + std::cout << "Results path: " << results_file_stem << std::endl; + results_file_stem += "results"; + BenchmarkResults results(max_lead_text_size, "benchmark-crud", results_file_stem.c_str()); Timer timer_total(Timer::type_UserTime); Timer timer(Timer::type_UserTime); @@ -259,4 +264,29 @@ int main() results.submit_single("crud_total_time", "Total time", "runtime_secs", timer_total); std::cout << "dummy = " << dummy << " (to avoid over-optimization)\n"; + + return 0; +} + +int main(int argc, const char** argv) +{ + if (argc > 1) { + std::string arg_path = argv[1]; + if (arg_path == "-h" || arg_path == "--help") { + std::cout << "Usage: " << argv[0] << " [-h|--help] [PATH]" << std::endl + << "Run the CRUD operations benchmark test application." << std::endl + << "Results are placed in the executable directory by default." << std::endl + << std::endl + << "Arguments:" << std::endl + << " -h, --help display this help" << std::endl + << " PATH alternate path to store the results files;" << std::endl + << " this path should end with a slash." << std::endl + << std::endl; + return 1; + } + } + + if (!initialize_test_path(argc, argv)) + return 1; + return benchmark_crud_main(); } diff --git a/test/object-store/CMakeLists.txt b/test/object-store/CMakeLists.txt index 21b97a960a8..05e3d135acc 100644 --- a/test/object-store/CMakeLists.txt +++ b/test/object-store/CMakeLists.txt @@ -140,6 +140,7 @@ if(NOT APPLE AND NOT WINDOWS_STORE) GIT_REPOSITORY https://github.com/libuv/libuv.git GIT_TAG ${libUV_Git_TAG} ) + # Don't use FetchContent_MakeAvailable since it wants to build libuv.so as well FetchContent_Populate(libuv) add_subdirectory(${libuv_SOURCE_DIR} ${libuv_BINARY_DIR} EXCLUDE_FROM_ALL) set(libuv_target uv_a) diff --git a/test/object-store/benchmarks/CMakeLists.txt b/test/object-store/benchmarks/CMakeLists.txt index 3c38559f7dd..058de20741c 100644 --- a/test/object-store/benchmarks/CMakeLists.txt +++ b/test/object-store/benchmarks/CMakeLists.txt @@ -36,6 +36,7 @@ target_include_directories(object-store-benchmarks PRIVATE if(REALM_ENABLE_SYNC) target_link_libraries(object-store-benchmarks SyncServer) + enable_stdfilesystem(object-store-benchmarks) endif() target_link_libraries(object-store-benchmarks ObjectStore Catch2::Catch2) @@ -44,3 +45,10 @@ set_target_properties(object-store-benchmarks PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD 1) add_dependencies(benchmarks object-store-benchmarks) + +# on Apple platforms we use the built-in CFRunLoop +# everywhere else it's libuv, except UWP where it doesn't build +if(NOT APPLE AND NOT WINDOWS_STORE) + target_include_directories(object-store-benchmarks PRIVATE ${libuv_include_dir}) + target_link_libraries(object-store-benchmarks ${libuv_target}) +endif() diff --git a/test/util/CMakeLists.txt b/test/util/CMakeLists.txt index 74891854ccf..ccb7aaba926 100644 --- a/test/util/CMakeLists.txt +++ b/test/util/CMakeLists.txt @@ -57,10 +57,8 @@ if(UNIX AND NOT APPLE) find_library(LIBRT rt) if(LIBRT) target_link_libraries(TestUtil ${LIBRT}) - else() + # Android has librt included in libc + elseif(NOT ANDROID) message(WARNING "librt was not found. This means that the benchmarks will not be able to link properly.") endif() endif() - - - diff --git a/tools/cmake/android.toolchain.cmake b/tools/cmake/android.toolchain.cmake new file mode 100644 index 00000000000..fda9e0fde34 --- /dev/null +++ b/tools/cmake/android.toolchain.cmake @@ -0,0 +1,16 @@ +if(CMAKE_GENERATOR STREQUAL Xcode) + message(FATAL_ERROR "Building for Android cannot use the Xcode generator.") +endif() + +# Callers can pick their own sysroot for packaging purposes, currently only needed for plain macosx builds +if(NOT DEFINED CMAKE_SYSTEM_NAME) + set(CMAKE_SYSTEM_NAME Android) +endif() + +# For some reason, APPLE is set when building for Android on MacOS +# This leads to the incorrect ar program being seleccted: "/usr/bin/ar" vs "llvm-ar" +# Unset APPLE now so the correct ar program is selected +# Remove once https://gitlab.kitware.com/cmake/cmake/-/issues/23333 is resolved +if(APPLE) + unset(APPLE) +endif() diff --git a/tools/cross_compile.sh b/tools/cross_compile.sh index 5ff3f0b8833..2ef59406733 100755 --- a/tools/cross_compile.sh +++ b/tools/cross_compile.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e @@ -56,9 +56,14 @@ if [ -z "${OS}" ] || [ -z "${BUILD_TYPE}" ]; then fi # Check for android-related obligatory fields -if [ "${OS}" == "android" ] && [ -z "${ARCH}" ]; then - echo "ERROR: option -a is needed for android builds"; - usage +if [[ "${OS}" == "android" ]]; then + if [[ -z "${ARCH}" ]]; then + echo "ERROR: option -a is needed for android builds"; + usage + elif [[ -z "${ANDROID_NDK}" ]]; then + echo "ERROR: set ANDROID_NDK to the top level path for the Android NDK"; + usage + fi fi if [ "${OS}" == "android" ]; then @@ -69,6 +74,7 @@ if [ "${OS}" == "android" ]; then -D CMAKE_INSTALL_PREFIX=install \ -D CMAKE_BUILD_TYPE="${BUILD_TYPE}" \ -D CMAKE_ANDROID_ARCH_ABI="${ARCH}" \ + -D CMAKE_TOOLCHAIN_FILE="./tools/cmake/android.toolchain.cmake" \ -D REALM_ENABLE_ENCRYPTION=1 \ -D REALM_VERSION="${VERSION}" \ -D CPACK_SYSTEM_NAME="Android-${ARCH}" \