Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General Cleanup #47

Merged
45 commits merged into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
2a42e7d
Removing MatX
mdemoret-nv Jun 21, 2022
ec6d022
Cleanup CMakeLists.txt
mdemoret-nv Jun 21, 2022
23deba5
Moving `make_pipeline` into pipeline.hpp
mdemoret-nv Jun 21, 2022
b4ee682
Forcing all `make_pipeline` to use the same function
mdemoret-nv Jun 21, 2022
95201c0
Cleaning up comments in `./python`
mdemoret-nv Jun 21, 2022
69caae0
Fixing benchmark tests filenames.
mdemoret-nv Jun 21, 2022
8142666
Merge branch 'branch-22.06' into mdd_cleanup
mdemoret-nv Jun 21, 2022
09fccc4
Adding YAPF to the CI python checks.
mdemoret-nv Jun 21, 2022
5b12925
Fixing up includes in the include folder
mdemoret-nv Jun 22, 2022
de3a77c
Fixing up the ordering of many includes
mdemoret-nv Jun 22, 2022
bed5ac5
More include fixes
mdemoret-nv Jun 22, 2022
10d7b5b
Removing architect and tests from being skipped by CI
mdemoret-nv Jun 22, 2022
ad3773c
Fixing #include "nvrpc" -> #include <nvrpc>
mdemoret-nv Jun 22, 2022
30dcb9a
Fixing missing pop visibility
mdemoret-nv Jun 22, 2022
c8861c7
Adding #pragma once checks to CI
mdemoret-nv Jun 22, 2022
757698b
Fixing missing `#pragma once`
mdemoret-nv Jun 22, 2022
c97c243
Combined `common_sinks.cpp` and `common_sources.cpp` into a single fi…
mdemoret-nv Jun 22, 2022
3f68bb1
Fixing compilation
mdemoret-nv Jun 22, 2022
9b7dab1
Removing files according to #48
mdemoret-nv Jun 22, 2022
c5762d4
IWYU fixes
mdemoret-nv Jun 22, 2022
203f954
Updating copyright
mdemoret-nv Jun 22, 2022
66fab36
Deleting more files according to #48
mdemoret-nv Jun 22, 2022
f8ecfe6
Reenabling test_segment.cpp
mdemoret-nv Jun 22, 2022
5368db2
Renaming `srf/internal` to `srf/engine`
mdemoret-nv Jun 22, 2022
52041ee
IWYU fixes
mdemoret-nv Jun 22, 2022
dceee6c
Fixing benchmarks compilation
mdemoret-nv Jun 22, 2022
b7d4e1e
Removing xtl and xtensor
mdemoret-nv Jun 24, 2022
bf9b27b
Incorporating feedback from reviews
mdemoret-nv Jun 24, 2022
1fc8c27
Updating includes in src
mdemoret-nv Jun 24, 2022
f5a7c68
Fixing includes of include/
mdemoret-nv Jun 24, 2022
8fe6569
More formatting cleanup.
mdemoret-nv Jun 24, 2022
12ab525
Style cleanup
mdemoret-nv Jun 24, 2022
77e3644
Style cleanup
mdemoret-nv Jun 24, 2022
d6cc5bf
Merge branch 'branch-22.06' into mdd_cleanup
mdemoret-nv Jun 24, 2022
aeec906
Converting all `#include <srf/...>` to `#include "srf/..."`
mdemoret-nv Jun 24, 2022
4f327ef
Removing the main header include separator comment
mdemoret-nv Jun 24, 2022
0fe16dc
Missed changing .h files
mdemoret-nv Jun 24, 2022
e09a070
Applying <> to "" rename to pysrf
mdemoret-nv Jun 24, 2022
32faaa8
Renaming tracer.cpp files to tracers.cpp to match module name
mdemoret-nv Jun 24, 2022
b5fe4df
Applying new formatting
mdemoret-nv Jun 24, 2022
029a127
Fixing up IWYU
mdemoret-nv Jun 24, 2022
0cbd808
Merge branch 'branch-22.06' into mdd_cleanup
mdemoret-nv Jun 24, 2022
f86d774
Getting code to compile again.
mdemoret-nv Jun 24, 2022
275289e
Getting code to compile again.
mdemoret-nv Jun 24, 2022
ec41107
Style cleanup
mdemoret-nv Jun 24, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,31 @@ ForEachMacros:
- foreach
- Q_FOREACH
- BOOST_FOREACH
IncludeBlocks: Preserve
IncludeBlocks: Regroup
IncludeCategories:
- Regex: '^<ext/.*\.h>'
# First find any headers in internal or public
- Regex: '^"(internal|public)\/.*\.(h|hpp)"'
Priority: 2
- Regex: '^<.*\.h>'
# Any other quoted includes need to be first (local to source)
- Regex: '^".*\.(h|hpp)"'
Priority: 1
- Regex: '^<.*'
Priority: 2
- Regex: '.*'
# Public SRF headers
- Regex: '^<srf\/.*\.(h|hpp)>'
Priority: 3
# Public NVRPC headers
- Regex: '^<nvrpc\/.*\.(h|hpp)>'
Priority: 4
# Last is system includes which dont have a '/' like <string> or <mutex>
- Regex: '<([a-z_])+>'
Priority: 6
# Finally, put all 3rd party includes before the system includes
- Regex: '^<.*'
Priority: 5
# IncludeIsMainSourceRegex: '$?'
IncludeIsMainRegex: '([-_](test|unittest))?$'
IndentCaseBlocks: false
IndentCaseLabels: false
IndentPPDirectives: None
IndentPPDirectives: BeforeHash
IndentWidth: 4
IndentWrappedFunctionNames: false
JavaScriptQuotes: Leave
Expand Down Expand Up @@ -147,7 +158,7 @@ SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Cpp11
Standard: c++17
StatementMacros:
- Q_UNUSED
- QT_REQUIRE_VERSION
Expand Down
83 changes: 37 additions & 46 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@ option(SRF_BUILD_DOCS "Enable building the python bindings for Srf" OFF)
option(SRF_BUILD_LIBRARY "Whether the entire SRF library should be built. If set to OFF, only the pieces needed for a target will be built. Set to ON if installing the library" ON)
option(SRF_BUILD_PYTHON "Enable building the python bindings for Srf" ON)
option(SRF_BUILD_TESTS "Whether or not to build SRF tests" ON)
option(SRF_ENABLE_MATX "Enables the header only library MatX. Requires CUDA 11.4." OFF)
option(SRF_ENABLE_XTENSOR "Enables the xtensor library." OFF)
option(SRF_USE_CCACHE "Enable caching compilation results with ccache" OFF)
option(SRF_USE_CLANG_TIDY "Enable running clang-tidy as part of the build process" OFF)
option(SRF_USE_CONDA "Enables finding dependencies via conda instead of vcpkg. Note: This will disable vcpkg. All dependencies must be installed first in the conda environment" OFF)
option(SRF_USE_CONDA "Enables finding dependencies via conda instead of vcpkg. Note: This will disable vcpkg. All dependencies must be installed first in the conda environment" ON)
option(SRF_USE_IWYU "Enable running include-what-you-use as part of the build process" OFF)

set(RAPIDS_VERSION "22.06" CACHE STRING "Which version of RAPIDs to build for. Sets default versions for RAPIDs CMake and RMM.")
Expand Down Expand Up @@ -144,57 +142,16 @@ include(cmake/setup_iwyu.cmake)
####################################
# - Begin SRF Targets --------------

# Keep all source files sorted
# Keep all source files sorted!!!
mdemoret-nv marked this conversation as resolved.
Show resolved Hide resolved
add_library(libsrf
src/public/benchmarking/trace_statistics.cpp
src/public/benchmarking/tracer.cpp
src/public/benchmarking/util.cpp
src/public/channel/channel.cpp
src/public/codable/encoded_object.cpp
src/public/core/addresses.cpp
src/public/core/bitmap.cpp
src/public/core/executor.cpp
src/public/core/fiber_pool.cpp
src/public/core/logging.cpp
# src/public/core/port_registry.cpp
src/public/cuda/device_guard.cpp
src/public/cuda/sync.cpp
src/public/manifold/manifold.cpp
src/public/metrics/counter.cpp
src/public/metrics/registry.cpp
src/public/memory/blob.cpp
src/public/memory/block.cpp
src/public/node/edge_registry.cpp
src/public/options/engine_groups.cpp
src/public/options/fiber_pool.cpp
src/public/options/options.cpp
src/public/options/placement.cpp
src/public/options/resources.cpp
src/public/options/services.cpp
src/public/options/topology.cpp
src/public/runnable/context.cpp
src/public/runnable/launcher.cpp
src/public/runnable/runnable.cpp
src/public/runnable/runner.cpp
src/public/segment/definition.cpp
# src/public/segment/runnable.cpp
src/public/runnable/types.cpp
src/public/utils/bytes_to_string.cpp
src/public/utils/thread_utils.cpp
src/public/utils/type_utils.cpp

# src/internal/data_plane/client_worker.cpp
# src/internal/data_plane/client.cpp
# src/internal/data_plane/instance.cpp
# src/internal/data_plane/server.cpp
src/internal/executor/executor.cpp
src/internal/executor/iexecutor.cpp
src/internal/pipeline/controller.cpp
src/internal/pipeline/instance.cpp
src/internal/pipeline/ipipeline.cpp
src/internal/pipeline/manager.cpp
src/internal/pipeline/pipeline.cpp
src/internal/pipeline/port_graph.cpp
src/internal/pipeline/manager.cpp
src/internal/pipeline/resources.cpp
src/internal/resources/device_resources.cpp
src/internal/resources/host_resources.cpp
Expand Down Expand Up @@ -236,6 +193,40 @@ add_library(libsrf
src/internal/utils/parse_config.cpp
src/internal/utils/parse_ints.cpp
src/internal/utils/shared_resource_bit_map.cpp
src/public/benchmarking/trace_statistics.cpp
src/public/benchmarking/tracer.cpp
src/public/benchmarking/util.cpp
src/public/channel/channel.cpp
src/public/codable/encoded_object.cpp
src/public/core/addresses.cpp
src/public/core/bitmap.cpp
src/public/core/executor.cpp
src/public/core/fiber_pool.cpp
src/public/core/logging.cpp
src/public/cuda/device_guard.cpp
src/public/cuda/sync.cpp
src/public/manifold/manifold.cpp
src/public/memory/blob.cpp
src/public/memory/block.cpp
src/public/metrics/counter.cpp
src/public/metrics/registry.cpp
src/public/node/edge_registry.cpp
src/public/options/engine_groups.cpp
src/public/options/fiber_pool.cpp
src/public/options/options.cpp
src/public/options/placement.cpp
src/public/options/resources.cpp
src/public/options/services.cpp
src/public/options/topology.cpp
src/public/runnable/context.cpp
src/public/runnable/launcher.cpp
src/public/runnable/runnable.cpp
src/public/runnable/runner.cpp
src/public/runnable/types.cpp
src/public/segment/definition.cpp
src/public/utils/bytes_to_string.cpp
src/public/utils/thread_utils.cpp
src/public/utils/type_utils.cpp
)

add_library(${PROJECT_NAME}::libsrf ALIAS libsrf)
Expand Down
1 change: 1 addition & 0 deletions benchmarks/bench_segment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <srf/benchmarking/util.hpp>
#include <srf/channel/status.hpp>
#include <srf/core/executor.hpp>
#include <srf/engine/pipeline/ipipeline.hpp>
#include <srf/node/rx_node.hpp>
#include <srf/node/rx_sink.hpp>
#include <srf/node/rx_source.hpp>
Expand Down
1 change: 0 additions & 1 deletion ci/conda/environments/dev_env_nogcc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ dependencies:
- scikit-build>=0.12
- spdlog=1.8.5
- ucx=1.12
- xtensor=0.24
- pip:
- cython
- flake8
Expand Down
3 changes: 0 additions & 3 deletions ci/conda/recipes/libsrf/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ requirements:
- scikit-build >=0.12
- spdlog 1.8.5
- ucx
- xtensor 0.24.*

outputs:
- name: libsrf
Expand Down Expand Up @@ -104,12 +103,10 @@ outputs:
- librmm {{ rapids_version }}
- nlohmann_json 3.9.*
- ucx
- xtensor 0.24.*
run:
# Manually add any packages necessary for run that do not have run_exports. Keep sorted!
- {{ pin_compatible('flatbuffers', max_pin='x.x')}}
- {{ pin_compatible('nlohmann_json', max_pin='x.x')}}
- {{ pin_compatible('xtensor', max_pin='x.x')}}
- {{ pin_compatible('ucx', max_pin='x.x')}}
- boost-cpp # Needed to use pin_run_as_build
test:
Expand Down
3 changes: 0 additions & 3 deletions ci/iwyu/mappings.imp
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,6 @@
# spdlog
{ "symbol": ["spdlog::details::file_helper::~file_helper", "private", "<spdlog/sinks/basic_file_sink.h>", "public"] },

# xtensor
{ "symbol": ["xt::no_ownership", "private", "<xtensor/xadapt.hpp>", "public"] },

# srf
{ "symbol": ["std::__decay_and_strip<std::shared_ptr<srf::TraceStatistics> &>::__type" , "private", "<srf/benchmarking/trace_statistics.hpp>", "public"] },

Expand Down
8 changes: 4 additions & 4 deletions ci/scripts/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ export PY_ROOT="python"
export PY_CFG="${PY_ROOT}/setup.cfg"
export PY_DIRS="${PY_ROOT} ci/scripts"

# work-around for known yapf issue https://github.com/google/yapf/issues/984
export YAPF_EXCLUDE_FLAGS="-e ${PY_ROOT}/versioneer.py -e ${PY_ROOT}/srf/_version.py"

# Determine the commits to compare against. If running in CI, these will be set. Otherwise, diff with main
export BASE_SHA=${CI_MERGE_REQUEST_DIFF_BASE_SHA:-${BASE_SHA:-main}}
export COMMIT_SHA=${CI_COMMIT_SHA:-${COMMIT_SHA:-HEAD}}
Expand All @@ -33,6 +30,7 @@ export PYTHON_FILE_REGEX='^(\.\/)?(?!\.|build).*\.(py|pyx|pxd)$'

# Use these options to skip any of the checks
export SKIP_COPYRIGHT=${SKIP_COPYRIGHT:-""}
export SKIP_PRAGMA_CHECK=${SKIP_PRAGMA_CHECK:-""}
export SKIP_CLANG_FORMAT=${SKIP_CLANG_FORMAT:-""}
export SKIP_CLANG_TIDY=${SKIP_CLANG_TIDY:-""}
export SKIP_IWYU=${SKIP_IWYU:-""}
Expand Down Expand Up @@ -68,7 +66,9 @@ function get_modified_files() {
local GIT_DIFF_BASE=${GIT_DIFF_BASE:-$(get_merge_base)}

# If invoked by a git-commit-hook, this will be populated
local result=( $(git diff ${GIT_DIFF_ARGS} $(get_merge_base) | grep -P ${1:-'.*'} | sed -e '/\/architect\//d' -e '/test_/d'))
# First filter only those that exist
# Next filter by provided regex
local result=( $(git diff ${GIT_DIFF_ARGS} $(get_merge_base) | xargs ls -1df 2>/dev/null | grep -P ${1:-'.*'} ))

if [[ "$__resultvar" ]]; then
eval $__resultvar="( ${result[@]} )"
Expand Down
26 changes: 25 additions & 1 deletion ci/scripts/cpp_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ if [[ "${SKIP_IWYU}" == "" && -z "${IWYU_TOOL}" ]]; then
fi

# Pre-populate the return values in case they are skipped
PRAGMA_CHECK_RETVAL=0
CLANG_TIDY_RETVAL=0
CLANG_FORMAT_RETVAL=0
IWYU_RETVAL=0
Expand All @@ -42,6 +43,16 @@ if [[ -n "${SRF_MODIFIED_FILES}" ]]; then
echo " $f"
done

# Check for `#pragma once` in any .hpp or .h file
if [[ "${SKIP_PRAGMA_CHECK}" == "" ]]; then
mdemoret-nv marked this conversation as resolved.
Show resolved Hide resolved

PRAGMA_CHECK_OUTPUT=`grep -rL --include=\*.{h,hpp} --exclude-dir={build,.cache} -e '#pragma once' $(get_modified_files $CPP_FILE_REGEX)`

if [[ -n "${PRAGMA_CHECK_OUTPUT}" ]]; then
PRAGMA_CHECK_RETVAL=1
fi
fi

# Clang-tidy
if [[ "${SKIP_CLANG_TIDY}" == "" ]]; then

Expand Down Expand Up @@ -69,7 +80,7 @@ if [[ -n "${SRF_MODIFIED_FILES}" ]]; then

# Include What You Use
if [[ "${SKIP_IWYU}" == "" ]]; then
IWYU_DIRS="benchmarks examples python src tools"
IWYU_DIRS="benchmarks python src tools"
NUM_PROC=$(get_num_proc)
IWYU_OUTPUT=`${IWYU_TOOL} -p ${BUILD_DIR} -j ${NUM_PROC} ${IWYU_DIRS} 2>&1`
IWYU_RETVAL=$?
Expand All @@ -79,6 +90,19 @@ else
fi

# Now check the values
if [[ "${SKIP_PRAGMA_CHECK}" != "" ]]; then
echo -e "\n\n>>>> SKIPPED: pragma check\n\n"
elif [[ "${PRAGMA_CHECK_RETVAL}" != "0" ]]; then
echo -e "\n\n>>>> FAILED: pragma check; begin output\n\n"
echo -e "Missing \`#pragma once\` in the following files:"
echo -e "${PRAGMA_CHECK_OUTPUT}"
echo -e "\n\n>>>> FAILED: pragma check; end output\n\n" \
"To auto-fix many issues (not all) run:\n" \
" ./ci/scripts/fix_all.sh\n\n"
else
echo -e "\n\n>>>> PASSED: pragma check\n\n"
fi

if [[ "${SKIP_CLANG_TIDY}" != "" ]]; then
echo -e "\n\n>>>> SKIPPED: clang-tidy check\n\n"
elif [[ "${CLANG_TIDY_RETVAL}" != "0" ]]; then
Expand Down
20 changes: 20 additions & 0 deletions ci/scripts/python_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ LANG=C.UTF-8
# Pre-populate the return values in case they are skipped
ISORT_RETVAL=0
FLAKE_RETVAL=0
YAPF_RETVAL=0

get_modified_files ${PYTHON_FILE_REGEX} SRF_MODIFIED_FILES

Expand All @@ -49,6 +50,11 @@ if [[ -n "${SRF_MODIFIED_FILES}" ]]; then
FLAKE_RETVAL=$?
fi

if [[ "${SKIP_YAPF}" == "" ]]; then
# Run yapf. Will return 1 if there are any diffs
YAPF_OUTPUT=`python3 -m yapf --style ${PY_CFG} --diff ${SRF_MODIFIED_FILES[@]} 2>&1`
YAPF_RETVAL=$?
fi
else
echo "No modified Python files to check"
fi
Expand Down Expand Up @@ -78,6 +84,20 @@ else
echo -e "\n\n>>>> PASSED: flake8 style check\n\n"
fi

if [[ "${SKIP_YAPF}" != "" ]]; then
echo -e "\n\n>>>> SKIPPED: yapf check\n\n"
elif [ "${YAPF_RETVAL}" != "0" ]; then
echo -e "\n\n>>>> FAILED: yapf style check; begin output\n\n"
echo -e "Incorrectly formatted files:"
YAPF_OUTPUT=`echo "${YAPF_OUTPUT}" | sed -nr 's/^\+\+\+ ([^ ]*) *\(reformatted\)$/\1/p'`
echo -e "${YAPF_OUTPUT}"
echo -e "\n\n>>>> FAILED: yapf style check; end output\n\n" \
"To auto-fix many issues (not all) run:\n" \
" ./ci/scripts/fix_all.sh\n\n"
else
echo -e "\n\n>>>> PASSED: yapf style check\n\n"
fi

RETVALS=(${ISORT_RETVAL} ${FLAKE_RETVAL})
IFS=$'\n'
RETVAL=`echo "${RETVALS[*]}" | sort -nr | head -n1`
Expand Down
17 changes: 0 additions & 17 deletions cmake/dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -148,23 +148,6 @@ include(deps/Configure_prometheus)
set(LIBCUDACXX_VERSION "1.6.0" CACHE STRING "Version of libcudacxx to use")
include(deps/Configure_libcudacxx)

# matx
# ====
if(SRF_ENABLE_MATX)
set(MATX_VERSION "0.1.0" CACHE STRING "Version of MatX to use")
include(deps/Configure_matx)
endif()

# xtensor
# =======
if(SRF_ENABLE_XTENSOR)
set(XTL_VERSION "0.7.3" CACHE STRING "Version of xtensor-stack/xtl to use")
include(deps/Configure_xtl)

set(XTENSOR_VERSION "0.24.0" CACHE STRING "Version of xtensor to use")
include(deps/Configure_xtensor)
endif()

if(SRF_BUILD_BENCHMARKS)
# google benchmark
# ================
Expand Down
Loading