Skip to content

Commit

Permalink
Fix multiple clanng-tidy warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
ljishen committed Dec 18, 2022
1 parent 464ed0e commit 5dad1f3
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 40 deletions.
4 changes: 2 additions & 2 deletions .vscode/c_cpp_properties.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"name": "linux-clang-arm64",
"cStandard": "c11",
"compileCommands": "${workspaceFolder}/build-aarch64/compile_commands.json",
"compilerPath": "/usr/bin/clang",
"compilerPath": "/usr/bin/clang++",
"configurationProvider": "ms-vscode.cmake-tools",
"cppStandard": "c++17",
"defines": [],
Expand All @@ -21,7 +21,7 @@
"name": "linux-clang-x64",
"cStandard": "c11",
"compileCommands": "${workspaceFolder}/build-x64/compile_commands.json",
"compilerPath": "/usr/bin/clang",
"compilerPath": "/usr/bin/clang++",
"configurationProvider": "ms-vscode.cmake-tools",
"cppStandard": "c++17",
"defines": [],
Expand Down
12 changes: 6 additions & 6 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@
"-isystem",
"/home/coder/opt/dpdk-v22.07-${buildKitTargetArch}/include",
"-isystem",
"/home/coder/projects/bitar/build-${buildKitTargetArch}/vcpkg_installed/arm64-linux/include",
"${workspaceFolder}/build-${buildKitTargetArch}/vcpkg_installed/arm64-linux/include",
"-isystem",
"/home/coder/projects/bitar/build-${buildKitTargetArch}/vcpkg_installed/x64-linux/include"
"${workspaceFolder}/build-${buildKitTargetArch}/vcpkg_installed/x64-linux/include"
],
"cmake.buildDirectory": "${workspaceFolder}/build-${buildKitTargetArch}",
"cmake.configureOnEdit": false,
"cmake.configureOnOpen": true,
"cmake.configureSettings": {
"BITAR_BUILD_APPS": true,
"BITAR_BUILD_ARROW": false,
"BITAR_BUILD_TESTS": true,
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
"CMAKE_TOOLCHAIN_FILE": "${workspaceFolder}/opt/${buildKitTargetArch}/src/vcpkg/scripts/buildsystems/vcpkg.cmake",
"ENABLE_DEVELOPER_MODE": true
"ENABLE_DEVELOPER_MODE": true,
"BITAR_BUILD_APPS": true,
"BITAR_BUILD_ARROW": false,
"BITAR_BUILD_TESTS": true
},
"cmake.defaultVariants": {
"buildType": {
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ set(VCPKG_ROOT
get_filename_component(_vcpkg_parent_dir "${VCPKG_ROOT}" DIRECTORY)
file(MAKE_DIRECTORY "${_vcpkg_parent_dir}")
# Use vcpkg for library management - should be called before defining project():
# https://github.com/microsoft/vcpkg/tree/2022.09.27#vcpkg-as-a-submodule. If
# https://github.com/microsoft/vcpkg/tree/2022.11.14#vcpkg-as-a-submodule. If
# updating vcpkg dependencies is required, use "-DENABLE_VCPKG_UPDATE:BOOL=ON"
run_vcpkg(VCPKG_DIR "${VCPKG_ROOT}" VCPKG_URL
"https://github.com/microsoft/vcpkg.git" ENABLE_VCPKG_UPDATE)

# Environment variable VCPKG_FORCE_SYSTEM_BINARIES must be set on arm, s390x,
# and ppc64le platforms before running the vcpkg toolchain that is invoked by
# the project command. See
# https://github.com/microsoft/vcpkg-tool/blob/2022-10-17/src/vcpkg.cpp#L281-L290
# https://github.com/microsoft/vcpkg-tool/blob/2022-12-14/src/vcpkg.cpp#L275-L284
# The variable CMAKE_HOST_SYSTEM_PROCESSOR cannot be used for checking the host
# system architecture, because it is set only after the project command.
if((machine_hardware_name_lower MATCHES "^arm"
Expand Down
2 changes: 1 addition & 1 deletion apps/app_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void InstallSignalHandler() {

std::uint32_t num_parallel_tests() {
// Reserve one of the lcores as the main lcore
static std::uint32_t kNumParallelTests = rte_lcore_count() - 1;
static const std::uint32_t kNumParallelTests = rte_lcore_count() - 1;
return kNumParallelTests;
}

Expand Down
10 changes: 5 additions & 5 deletions apps/demo_app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ arrow::Result<std::shared_ptr<arrow::Buffer>> ReadRawData(
const std::shared_ptr<arrow::io::MemoryMappedFile>& file,
std::int64_t num_bytes_want) {
ARROW_ASSIGN_OR_RAISE(auto file_size, file->GetSize());
std::int64_t num_bytes_to_read = GetNumBytesToRead(num_bytes_want, file_size);
const std::int64_t num_bytes_to_read = GetNumBytesToRead(num_bytes_want, file_size);

ARROW_ASSIGN_OR_RAISE(
auto buffer,
Expand Down Expand Up @@ -155,7 +155,7 @@ arrow::Result<std::shared_ptr<arrow::Buffer>> SerializeTable(
arrow::ipc::MakeStreamWriter(&mock_output_stream, table->schema(), write_options));
ARROW_RETURN_NOT_OK(mock_writer->WriteTable(*table));
ARROW_RETURN_NOT_OK(mock_writer->Close());
std::int64_t table_size = mock_output_stream.GetExtentBytesWritten();
const std::int64_t table_size = mock_output_stream.GetExtentBytesWritten();

std::shared_ptr<arrow::Buffer> serialized_table;

Expand Down Expand Up @@ -264,7 +264,7 @@ bool WaitForAsyncCompletion(
for (std::uint32_t idx = 0; idx < num_parallel_tests(); ++idx) {
const auto& device = devices[device_id];

int ret = rte_eal_wait_lcore(device->LcoreOf(queue_pair_id));
const int ret = rte_eal_wait_lcore(device->LcoreOf(queue_pair_id));
if (ret == 0) {
RTE_LOG(ERR, USER1,
"Unable to start async operation for queue pair %hu of compress device "
Expand Down Expand Up @@ -402,7 +402,7 @@ arrow::Status BenchmarkCompressAsync(
Advance(device_id, queue_pair_id, device->num_qps());
}

bool async_success = WaitForAsyncCompletion(devices);
const bool async_success = WaitForAsyncCompletion(devices);

// Avoid missing the opportunity to recycle by waiting till results from all worker
// lcores are known
Expand Down Expand Up @@ -468,7 +468,7 @@ arrow::Status BenchmarkDecompressAsync(
Advance(device_id, queue_pair_id, device->num_qps());
}

bool async_success = WaitForAsyncCompletion(devices);
const bool async_success = WaitForAsyncCompletion(devices);

if (!async_success) {
return arrow::Status::IOError("Failed to complete async decompression");
Expand Down
2 changes: 1 addition & 1 deletion cmake_modules/FindArrow.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ else()
"Use the Arrow library from the git repository for building when needed"
)
set(BITAR_ARROW_GIT_TAG
"c1aef6ffa5efbb4984888ce3471082383a098829"
"7e5730f4f64c9e6229840b439832e90cc84489ee"
CACHE
STRING
"Use the source at the git branch, tag or commit hash from the Arrow repository for building when needed"
Expand Down
12 changes: 6 additions & 6 deletions scripts/install-deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ if [[ -d "$IWYU_SOURCE_DIR" ]]; then
git fetch origin clang_$LLVM_VERSION &&
git reset --hard origin/clang_$LLVM_VERSION) ||
(echo "[INFO] Failed to update the source repository. Removing it before re-cloning..." &&
cd .. && rm --force --recursive "$IWYU_SOURCE_DIR")
cd .. &&
rm --force --recursive "$IWYU_SOURCE_DIR")
fi

if ! [[ -d "$IWYU_SOURCE_DIR" ]]; then
Expand All @@ -206,18 +207,17 @@ if ! [[ -d "$IWYU_SOURCE_DIR" ]]; then
"$IWYU_SOURCE_DIR" && cd "$IWYU_SOURCE_DIR"
fi

IWYU_BUILD_DIRNAME=build
IWYU_BUILD_DIRNAME=build-$MACHINE_HARDWARE_NAME
CURRENT_COMMIT_HASH=$(git rev-parse HEAD)
if ! [[ -x "$IWYU_INSTALL_DIR"/bin/include-what-you-use ]] ||
[[ "${OLD_COMMIT_HASH:-}" != "$CURRENT_COMMIT_HASH" ]]; then
rm --force --recursive "$IWYU_BUILD_DIRNAME" &&
mkdir "$IWYU_BUILD_DIRNAME" && cd "$IWYU_BUILD_DIRNAME"
cmake -G "Unix Makefiles" \
cmake .. -G Ninja \
-DCMAKE_PREFIX_PATH="/usr/lib/llvm-$LLVM_VERSION" \
-DCMAKE_INSTALL_PREFIX="$IWYU_INSTALL_DIR" ..
-DCMAKE_INSTALL_PREFIX="$IWYU_INSTALL_DIR"
# Generate executable "$IWYU_INSTALL_DIR"/bin/include-what-you-use
cmake --build . -j
cmake --install .
ninja install
else
echo "[INFO] Nothing to update"
fi
Expand Down
3 changes: 3 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ target_link_system_libraries(${PROJECT_NAME} PRIVATE absl::span fmt::fmt
# Force the use of compile-time format string checks:
# https://fmt.dev/latest/api.html#compile-time-format-string-checks
target_compile_definitions(${PROJECT_NAME} PRIVATE FMT_ENFORCE_COMPILE_STRING)
# Optimize 'enum_switch' to have O(1) complexity
# https://github.com/Neargye/magic_enum/releases/tag/v0.8.2
target_compile_definitions(${PROJECT_NAME} PRIVATE MAGIC_ENUM_ENABLE_HASH)

set(PUBLIC_DEPENDENCIES "dpdk ${dpdk_minimum_required_version}")
foreach(DEPENDENCY ${PUBLIC_DEPENDENCIES})
Expand Down
14 changes: 7 additions & 7 deletions src/driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include <vector>

#include <magic_enum.hpp>
#include <magic_enum_switch.hpp>

#include "include/device.h"
#include "include/util.h"
Expand All @@ -55,8 +56,8 @@ arrow::Status HasDeviceIds(const std::vector<std::uint8_t>& avail_dev_ids,
const std::vector<std::uint8_t>& device_ids) {
std::vector<std::uint8_t> not_avail_dev_ids;
for (const auto& device_id : device_ids) {
bool exists = std::find(std::begin(avail_dev_ids), std::end(avail_dev_ids),
device_id) != std::end(avail_dev_ids);
const bool exists = std::find(std::begin(avail_dev_ids), std::end(avail_dev_ids),
device_id) != std::end(avail_dev_ids);
if (!exists) {
not_avail_dev_ids.emplace_back(device_id);
}
Expand Down Expand Up @@ -87,14 +88,13 @@ arrow::Result<T> GetPCIId(const char* device_name) {
internal::ReadFileContent(fmt::format(
FMT_STRING("/sys/class/infiniband/{}/device/{}"), device_name, type_name)));

try {
auto pci_id_opt = magic_enum::enum_cast<T>(
static_cast<std::uint16_t>(std::stoi(pci_id_str, nullptr, 0)));
return pci_id_opt.value();
} catch (...) {
auto pci_id_opt = magic_enum::enum_cast<T>(
static_cast<std::uint16_t>(std::stoi(pci_id_str, nullptr, 0)));
if (!pci_id_opt.has_value()) {
return arrow::Status::NotImplemented("Compress device with ", type_name, "_id '",
pci_id_str, "' is not supported.");
}
return pci_id_opt.value();
}

arrow::Result<std::vector<std::unique_ptr<MLX5CompressDevice>>> CreateDevices(
Expand Down
8 changes: 4 additions & 4 deletions src/memory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,12 @@ arrow::Status QueuePairMemory<Class, Enable>::Preallocate() {
// A burst contains src mbufs and dst mbufs
auto num_mbufs_in_burst = static_cast<std::uint32_t>(
configuration_->burst_size() * configuration_->max_sgl_segs() * 2);
std::uint32_t mbuf_pool_cache_size =
const std::uint32_t mbuf_pool_cache_size =
RTE_MIN(num_mbufs_in_burst * kCacheSizeAmplifier,
static_cast<std::uint32_t>(RTE_MEMPOOL_CACHE_MAX_SIZE));
std::uint32_t num_inflight_mbufs =
const std::uint32_t num_inflight_mbufs =
RTE_MIN(num_mbufs_in_burst * kMaxInflightOps, kMaxInflightMbufs);
std::uint32_t num_mbufs_in_pool =
const std::uint32_t num_mbufs_in_pool =
num_inflight_mbufs + mbuf_pool_cache_size + num_mbufs_in_burst;

auto mbuf_pool_name =
Expand All @@ -265,7 +265,7 @@ arrow::Status QueuePairMemory<Class, Enable>::Preallocate() {
auto operation_pool_cache_size = static_cast<std::uint32_t>(RTE_MIN(
configuration_->burst_size() * kCacheSizeAmplifier, RTE_MEMPOOL_CACHE_MAX_SIZE));
// Round up division of `(num_mbufs_in_pool / 2) / configuration_->max_sgl_segs()`
std::uint32_t num_ops_in_pool =
const std::uint32_t num_ops_in_pool =
(num_mbufs_in_pool / 2 + configuration_->max_sgl_segs() - 1) /
configuration_->max_sgl_segs();

Expand Down
2 changes: 1 addition & 1 deletion src/memory_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class RtememzoneAllocator {
return arrow::Status::OK();
}

fmt::format_int name(rte_rdtsc());
const fmt::format_int name(rte_rdtsc());
const auto* memzone = rte_memzone_reserve_aligned(
name.c_str(), static_cast<std::size_t>(size),
static_cast<std::int32_t>(rte_socket_id()), RTE_MEMZONE_IOVA_CONTIG,
Expand Down
12 changes: 7 additions & 5 deletions vcpkg.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json",
"$schema": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
"name": "bitar",
"homepage": "https://github.com/ljishen/bitar",
"description": "Bitar is a C++ library to simplify accessing hardware compression/decompression accelerators.",
"builtin-baseline": "e82778a710538de07955dea6eb785132c3804d63",
"builtin-baseline": "71f51b100be2b5d32e3907572d99dc2f97088c8d",
"dependencies": [
{
"name": "abseil",
Expand All @@ -26,10 +26,12 @@
},
{
"name": "magic-enum",
"version>=": "0.8.1"
"version>=": "0.8.2"
},
{
"$explanation": ["Dependency for system-installed Apache Arrow library and DPDK"],
"$explanation": [
"Dependency for system-installed Apache Arrow library and DPDK"
],
"name": "openssl"
}
],
Expand All @@ -54,7 +56,7 @@
"dependencies": [
{
"name": "catch2",
"version>=": "3.2.0"
"version>=": "3.2.1"
}
]
}
Expand Down

0 comments on commit 5dad1f3

Please sign in to comment.