Skip to content

Commit

Permalink
Add Benchmark CLI for format comparison and perf tracking (#329)
Browse files Browse the repository at this point in the history
* Add version API and pull version w/ hash during build

* Move gtest up, and update it; Add new dependencies for ion-bench tool

* Update tests and build for updated gtest; Add option to not include tests in build

* Add ion-bench tool for performing (de-)serialization benchmarks

* Remove unneeded files

* Update cmake to dial in c++ standard requirements, and update gha workflow to account for changes better

* Fix job title for ubuntu/mac; add git config to fix dubious permissions issue

* Update actions/checkout, see if fetch-tags works to fix git describe issue

* Roll back to checkout@v3, v4 was too new for amazonlinux 1&2; Add fetch-depth

* Update build for CodeQL

* Remove --system, job is not containerized

* Fix memleak of ION_INT digits when closing or resetting a reader

* Initialize ion_decimal, fixes crash during ion_decimal_free when uninitialized data sets decimal to NUMBER type

* Put verbose debug output behind a cmake flag

* Address feedback; Fix typos and remove commented code
  • Loading branch information
nirosys authored Oct 16, 2023
1 parent 94f2d70 commit cf747d3
Show file tree
Hide file tree
Showing 47 changed files with 3,179 additions and 107 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
submodules: recursive
fetch-tags: true
fetch-depth: 50

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
Expand All @@ -64,6 +66,8 @@ jobs:
# and modify them (or add more) to build your code if your project
# uses a compiled language

- name: Create git config
run: git config --add safe.directory $GITHUB_WORKSPACE
- name: Build
run: ./build-release.sh

Expand Down
134 changes: 75 additions & 59 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,101 +8,117 @@ on:

jobs:
test_amazon_linux:
name: ${{ format('Test ({0}, {1})', matrix.container-image, matrix.compiler.cc)}}
name: ${{ format('Test ({0}, {1})', matrix.image, matrix.compiler.cc)}}
strategy:
# Run all jobs, even if one fails. This makes it easier to gather debugging info for various platforms.
fail-fast: false
matrix:
container-image: ['amazonlinux:1', 'amazonlinux:2']
compiler:
- { cc: 'gcc', cxx: 'g++', debug_cflags: '', debug_cxxflags: '', debug_ldflags: '' }
- { cc: 'clang', cxx: 'clang++', debug_cflags: '', debug_cxxflags: '', debug_ldflags: '' }
image: ['amazonlinux:1', 'amazonlinux:2']
toolchain: [ "gcc", "clang" ]
include:
- image: amazonlinux:2
toolchain: gcc
compiler: { cc: 'gcc', cxx: 'g++', packages: 'gcc gcc-c++' }
- image: amazonlinux:2
toolchain: clang
compiler: { cc: 'clang', cxx: 'clang++', packages: 'clang gcc10-c++ compiler-rt' }
# amazonlinux:1's 'gcc'/'gcc-c++' packages install 4.8.5 which do not support C++14 which is needed
# by our googletest version. The clang package installs 3.6.2, which supports C++14, but does not support
# the sanitizers we want to use to capture issues during unit testing.
- image: amazonlinux:1
toolchain: gcc
compiler: { cc: 'gcc', cxx: 'g++', packages: "gcc72 gcc72-c++" }
# Need to set the target for clang, since the default target it is configured with differs from the gcc target installed that it depends on.
- image: amazonlinux:1
toolchain: clang
compiler: { cc: 'clang', cxx: 'clang++', packages: 'clang6.0', cxxflags: "-target x86_64-amazon-linux", cflags: "-target x86_64-amazon-linux" }
runs-on: ubuntu-latest
container: ${{ format('docker://{0}', matrix.container-image) }}
container: ${{ matrix.image }}
env:
CC: ${{ matrix.compiler.cc }}
CXX: ${{ matrix.compiler.cxx }}
DEBUG_CFLAGS: ${{ matrix.compiler.debug_cflags }}
DEBUG_CXXFLAGS: ${{ matrix.compiler.debug_cxxflags }}
DEBUG_LDFLAGS: ${{ matrix.compiler.debug_ldflags }}
CFLAGS: ${{ matrix.compiler.cflags }}
CXXFLAGS: ${{ matrix.compiler.cxxflags }}
LDFLAGS: ${{ matrix.compiler.ldflags }}
UBSAN_OPTIONS: "print_stacktrace=1"
ASAN_OPTIONS: "halt_on_error=0"
steps:
- name: Install `which`
run: yum install which -y
- name: Install `git`
# Amazon Linux needs a newer version of git installed for actions/checkout@v2
run: yum install git -y
- name: Install `make`
run: yum install make -y
- name: Install `cmake3`
# Amazon Linux needs a newer version of git installed for actions/checkout@v2
- name: Install Dependencies
run: |
yum install cmake3 -y
yum install which git make cmake3 -y
ln -s `which cmake3` /usr/bin/cmake
- name: Install `clang`
if: matrix.compiler.cc == 'clang'
run: yum install clang -y
- name: Install `gcc`
if: matrix.compiler.cc == 'gcc'
run: yum install gcc -y
- name: Install `g++`
run: yum install gcc-c++ -y
- name: Install ${{ matrix.compiler.cc }}
run: yum install ${{ matrix.compiler.packages }} -y
- name: Checkout Code
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
submodules: recursive
- name: Build
run: |
./build-debug.sh
./build-release.sh
- name: Test
run: |
./build/debug/test/all_tests
./build/release/test/all_tests
fetch-tags: true
fetch-depth: 50 # we need to be able to fetch the tag that is nearest to the current commit.
- name: Create git config # Fixes an issue where git refuses to work due to dubious permissions.
run: git config --system --add safe.directory $GITHUB_WORKSPACE
- name: Build Debug
run: ./build-debug.sh
- name: Test Debug
run: ./build/debug/test/all_tests
- name: Build Release
env:
CMAKE_FLAGS: -DIONC_BENCHMARKING_ENABLED=on
run: ./build-release.sh
- name: Test Release
run: ./build/release/test/all_tests

test_ubuntu_and_mac:
name: ${{ format('Test ({0}, {1})', matrix.os, matrix.compiler.cc)}}
name: ${{ format('Test ({0}, {1})', matrix.image, matrix.toolchain)}}
strategy:
# Run all jobs, even if one fails. This makes it easier to gather debugging info for various platforms.
fail-fast: false
matrix:
os: ['macos-latest', 'ubuntu-latest']
compiler:
- { cc: 'gcc', cxx: 'g++', debug_cflags: '-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls', debug_cxxflags: '-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls', debug_ldflags: '-fsanitize=address,undefined' }
- { cc: 'clang', cxx: 'clang++', debug_cflags: '-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls', debug_cxxflags: '-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls', debug_ldflags: '' }
# gcc just redirects to clang on the macos vm; replace with a specific gcc alias
exclude:
- os: 'macos-latest'
compiler: { cc: 'gcc', cxx: 'g++'}
image: ['macos-latest', 'ubuntu-latest']
toolchain: ['gcc', 'clang']
include:
- os: 'macos-latest'
compiler: { cc: 'gcc-11', cxx: 'g++-11', debug_cflags: '-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls', debug_cxxflags: '-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls', debug_ldflags: '-fsanitize=address,undefined' }
runs-on: ${{ matrix.os }}
- toolchain: clang
compiler: { cc: 'clang', cxx: 'clang++' }
- toolchain: gcc
compiler: { cc: 'gcc', cxx: 'g++' }
- image: 'macos-latest'
toolchain: gcc
compiler: { cc: 'gcc-11', cxx: 'g++-11' }
runs-on: ${{ matrix.image }}
env:
CC: ${{ matrix.compiler.cc }}
CXX: ${{ matrix.compiler.cxx }}
DEBUG_CFLAGS: ${{ matrix.compiler.debug_cflags }}
DEBUG_CXXFLAGS: ${{ matrix.compiler.debug_cxxflags }}
DEBUG_LDFLAGS: ${{ matrix.compiler.debug_ldflags }}
CFLAGS: ${{ matrix.compiler.cflags }}
CXXFLAGS: ${{ matrix.compiler.cxxflags }}
LDFLAGS: ${{ matrix.compiler.ldflags }}
UBSAN_OPTIONS: "print_stacktrace=1"
ASAN_OPTIONS: "halt_on_error=0"
steps:
- name: Checkout Code
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
submodules: recursive
fetch-tags: true
fetch-depth: 50
- name: Create git config # Fixes an issue where git refuses to work due to dubious permissions.
run: git config --global --add safe.directory $GITHUB_WORKSPACE
# This step should fix an issue with building on macos with gcc, and xcode 14.0,
# and should be removed once 14.1 is made default.
- name: Fix for XCode 14.0
if: runner.os == 'macOS'
run: |
echo "DEVELOPER_DIR=/Applications/Xcode_14.1.app" >> $GITHUB_ENV
- name: Build
run: |
./build-debug.sh
./build-release.sh
- name: Test
run: |
./build/debug/test/all_tests
./build/release/test/all_tests
- name: Build Debug
id: build_debug
run: ./build-debug.sh
- name: Test Debug
run: ./build/debug/test/all_tests
- name: Build Release
id: build_release
run: ./build-release.sh
- name: Test Release
run: ./build/release/test/all_tests

documentation:
name: Generate Documentation
Expand Down
20 changes: 17 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
[submodule "ion-tests"]
path = ion-tests
url = https://github.com/amazon-ion/ion-tests.git
[submodule "googletest"]
path = test/googletest
[submodule "tools/ion-bench/deps/libcbor"]
path = tools/ion-bench/deps/libcbor
url = https://github.com/PJK/libcbor.git
[submodule "tools/ion-bench/deps/json-c"]
path = tools/ion-bench/deps/json-c
url = https://github.com/json-c/json-c.git
[submodule "tools/ion-bench/deps/msgpack-c"]
path = tools/ion-bench/deps/msgpack-c
url = https://github.com/msgpack/msgpack-c.git
[submodule "tools/ion-bench/deps/yyjson"]
path = tools/ion-bench/deps/yyjson
url = https://github.com/ibireme/yyjson.git
[submodule "tools/ion-bench/deps/google-benchmark"]
path = tools/ion-bench/deps/google-benchmark
url = https://github.com/google/benchmark.git
[submodule "deps/google-test"]
path = deps/google-test
url = https://github.com/google/googletest.git
ignore = untracked
72 changes: 64 additions & 8 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
cmake_minimum_required(VERSION 3.6 FATAL_ERROR)

project(IonC
VERSION 1.1.2
LANGUAGES CXX C)
Expand All @@ -17,23 +16,68 @@ else()
add_compile_options("$<$<CONFIG:DEBUG>:-O0;-g3;-ggdb3;-Wall>")
endif()

set(IONC_FORCE_SANITIZERS OFF CACHE BOOL "Force use of compiler sanitizers")
set(IONC_ENABLE_VERBOSE_DEBUG OFF CACHE BOOL "Enable verbose logging for ion-c")

## Build Type Settings
if (CMAKE_BUILD_TYPE STREQUAL "Release")
add_compile_definitions(NDEBUG=1)
elseif (CMAKE_BUILD_TYPE STREQUAL "Debug")
include(CheckCXXCompilerFlag)
# check_cxx_compiler_flag doesn't just check compiler flags, it also attempts to link the program, which
# in this case, also requires sanitizer flags. So we have to abuse CMAKE_REQUIRED_LINK_OPTIONS since that's
# the only lever we're given afaict. This does not work with cmake prior to 3.14.
list(INSERT CMAKE_REQUIRED_LINK_OPTIONS 0 "-fsanitize=address,undefined")
check_cxx_compiler_flag("-g -fsanitize=address,undefined -fno-omit-frame-pointer" UBISAN_OK)
list(REMOVE_AT CMAKE_REQUIRED_LINK_OPTIONS 0)
if (UBISAN_OK OR IONC_FORCE_SANITIZERS)
add_compile_options(
-fsanitize=address,undefined
-fsanitize-recover=address
)
add_link_options(-fsanitize=address,undefined -fsanitize-recover=address)
endif()
add_compile_options(-g -fno-omit-frame-pointer -fno-optimize-sibling-calls)
if (IONC_ENABLE_VERBOSE_DEBUG)
add_compile_definitions(DEBUG=1)
endif()
elseif (CMAKE_BUILD_TYPE STREQUAL "Profiling")
add_compile_options(
-O3 -march=native
-fno-omit-frame-pointer
-fno-optimize-sibling-calls
-g
)
add_compile_definitions(NDEBUG=1)
set(IONC_BENCHMARKING_ENABLED ON CACHE BOOL "Enable Benchmarking")
endif()

## ion-c Build Version
set(IONC_FULL_VERSION ${CMAKE_PROJECT_VERSION})
find_program(GIT_EXECUTABLE "git")
add_custom_target(
version
${CMAKE_COMMAND} -D SRC=${CMAKE_SOURCE_DIR}/build_version.h.in
-D DST=${CMAKE_BINARY_DIR}/build_version.h
-D GIT_EXECUTABLE=${GIT_EXECUTABLE}
-P ${CMAKE_SOURCE_DIR}/cmake/VersionHeader.cmake
)

include(GNUInstallDirs)

set(IONC_DECIMAL_NUM_DIGITS "34" CACHE STRING "Number of digits supported without added allocation")
option(IONC_BUILD_TESTS "Enable or Disable building of ion-c tests" ON)

# NOTE: DECNUMDIGITS must be set across all compilation units to at least DECQUAD_Pmax (34), so that the value is
# guaranteed to be consistent between ionc and decNumber. This is required for conversions between decQuad and
# decNumber. This is NOT the limit on decimal precision; ION_DECIMAL can handle arbitrarily large precision.
add_definitions(-DDECNUMDIGITS=34)
message(STATUS "Setting DECNUMBER max digits to 34")
add_definitions(-DDECNUMDIGITS=${IONC_DECIMAL_NUM_DIGITS})
message(STATUS "Setting DECNUMBER max digits to ${IONC_DECIMAL_NUM_DIGITS}")

set(CMAKE_INSTALL_RPATH "$ORIGIN")
set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

add_subdirectory(decNumber)
add_subdirectory(ionc)
add_subdirectory(test)
add_subdirectory(tools)

export(PACKAGE IonC)

include(cmake/CMakeCPack.cmake)
Expand All @@ -59,3 +103,15 @@ configure_file( "${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_uninstall.cmake.in"
add_custom_target(uninstall
COMMAND ${CMAKE_COMMAND} -P
"${CMAKE_CURRENT_BINARY_DIR}/cmake/cmake_uninstall.cmake" )


add_subdirectory(decNumber)
add_subdirectory(ionc)
add_subdirectory(tools)
if (IONC_BUILD_TESTS)
if (NOT TARGET gtest_main)
message("Using included google-test")
add_subdirectory(deps/google-test EXCLUDE_FROM_ALL)
endif()
add_subdirectory(test)
endif()
14 changes: 7 additions & 7 deletions build-debug.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

set -x

DEFAULT_CFLAGS="-fsanitize=address,undefined -fsanitize-recover=address -fno-omit-frame-pointer -fno-optimize-sibling-calls"
DEFAULT_CXXFLAGS="${DEFAULT_CFLAGS}"
DEFAULT_LDFLAGS="-fsanitize=address,undefined"

export LD="$CXX"

if ! [ -x "$(command -v cmake)" ]; then
echo 'Error: cmake is not installed.' >&2
exit 1
fi

mkdir -p build/debug && cd build/debug
cmake \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_C_FLAGS_DEBUG="${DEBUG_CFLAGS-${DEFAULT_CFLAGS}}" \
-DCMAKE_CXX_FLAGS_DEBUG="${DEBUG_CXXFLAGS-${DEFAULT_CXXFLAGS}}" \
${CMAKE_FLAGS} \
../..
make clean && make LDFLAGS="${DEBUG_LDFLAGS-${DEFAULT_LDFLAGS}}" -j"$(nproc || sysctl -n hw.ncpu)"
make clean && make -j"$(nproc || sysctl -n hw.ncpu)"
12 changes: 10 additions & 2 deletions build-release.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/bin/sh --

set -x

export LD="$CXX"

mkdir -p build/release
cd build/release

Expand All @@ -8,5 +12,9 @@ if ! [ -x "$(command -v cmake)" ]; then
exit 1
fi

cmake -DCMAKE_BUILD_TYPE=Release ../..
make clean && make
cmake \
-DCMAKE_BUILD_TYPE=Release \
${CMAKE_FLAGS} \
../..

make clean && make -j"$(nproc || sysctl -n hw.ncpu)"
9 changes: 9 additions & 0 deletions build_version.h.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#ifndef IONC_VERSION_H__
#define IONC_VERSION_H__
#define IONC_BUILD_VERSION "@IONC_VERSION@ (rev: @IONC_VERSION_HASH@)"
#define IONC_VERSION "@IONC_VERSION@"
#define IONC_VERSION_MAJOR @IONC_VERSION_MAJOR@
#define IONC_VERSION_MINOR @IONC_VERSION_MINOR@
#define IONC_VERSION_PATCH @IONC_VERSION_PATCH@
#define IONC_VERSION_HASH "@IONC_VERSION_HASH@"
#endif /* IONC_VERSION_H__ */
Loading

0 comments on commit cf747d3

Please sign in to comment.