From ad04349f54468dadd5009ab85deec20630e0124a Mon Sep 17 00:00:00 2001 From: Vivek Panyam Date: Tue, 22 Sep 2020 01:04:35 -0400 Subject: [PATCH 1/2] [Build] Refactor backends to adhere to stricter build warnings --- source/bazel/cc.bzl | 6 +++++ source/neuropod/backends/BUILD | 3 ++- source/neuropod/backends/neuropod_backend.cc | 4 +-- source/neuropod/backends/python_bridge/BUILD | 5 ++-- source/neuropod/backends/tensorflow/BUILD | 5 ++-- .../backends/tensorflow/tf_backend.cc | 4 +-- .../neuropod/backends/tensorflow/tf_tensor.cc | 8 +++--- .../neuropod/backends/tensorflow/tf_tensor.hh | 12 ++++----- source/neuropod/backends/torchscript/BUILD | 6 +++-- .../backends/torchscript/torch_backend.cc | 24 ++++++++--------- .../backends/torchscript/torch_tensor.cc | 26 +++++++++++++++++++ .../backends/torchscript/torch_tensor.hh | 26 ++++++++++--------- .../neuropod/internal/backend_registration.hh | 2 +- 13 files changed, 85 insertions(+), 46 deletions(-) create mode 100644 source/neuropod/backends/torchscript/torch_tensor.cc diff --git a/source/bazel/cc.bzl b/source/bazel/cc.bzl index 4c560959..f91af69c 100644 --- a/source/bazel/cc.bzl +++ b/source/bazel/cc.bzl @@ -17,6 +17,12 @@ _warnings = [ # We're okay with these for now, but should refactor to remove if we can "-Wno-global-constructors", "-Wno-exit-time-destructors", + + # It's okay if we use `default` in switch statements + "-Wno-switch-enum", + + # Flexible array members + "-Wno-c99-extensions", ] _test_warnings = _warnings + [ diff --git a/source/neuropod/backends/BUILD b/source/neuropod/backends/BUILD index 975c0093..898b0c30 100644 --- a/source/neuropod/backends/BUILD +++ b/source/neuropod/backends/BUILD @@ -13,8 +13,9 @@ # limitations under the License. load("@bazel_tools//tools/build_defs/pkg:pkg.bzl", "pkg_tar") +load("//bazel:cc.bzl", "neuropod_cc_library") -cc_library( +neuropod_cc_library( name = "neuropod_backend", srcs = [ "neuropod_backend.cc", diff --git a/source/neuropod/backends/neuropod_backend.cc b/source/neuropod/backends/neuropod_backend.cc index cdcf48ff..7574314c 100644 --- a/source/neuropod/backends/neuropod_backend.cc +++ b/source/neuropod/backends/neuropod_backend.cc @@ -129,7 +129,7 @@ void validate_tensors_against_specs(const NeuropodValueMap & tensors, } // Validate the shape - for (int i = 0; i < spec.dims.size(); i++) + for (size_t i = 0; i < spec.dims.size(); i++) { auto dim = tensor->get_dims()[i]; auto expected = spec.dims[i]; @@ -318,7 +318,7 @@ std::unique_ptr NeuropodBackend::infer_internal(const Neuropod return out; } -std::unique_ptr NeuropodBackend::infer_internal(const NeuropodValueMap &inputs) +std::unique_ptr NeuropodBackend::infer_internal(const NeuropodValueMap & /*unused*/) { NEUROPOD_ERROR("Backend implementations must provide a `infer_internal` implementation"); } diff --git a/source/neuropod/backends/python_bridge/BUILD b/source/neuropod/backends/python_bridge/BUILD index 846ae992..8ca14817 100644 --- a/source/neuropod/backends/python_bridge/BUILD +++ b/source/neuropod/backends/python_bridge/BUILD @@ -13,8 +13,9 @@ # limitations under the License. load("@bazel_tools//tools/build_defs/pkg:pkg.bzl", "pkg_tar") +load("//bazel:cc.bzl", "neuropod_cc_binary", "neuropod_cc_library") -cc_binary( +neuropod_cc_binary( name = "libneuropod_pythonbridge_backend.so", srcs = [ "//neuropod:libneuropod.so", @@ -33,7 +34,7 @@ cc_binary( ], ) -cc_library( +neuropod_cc_library( name = "python_bridge", srcs = [ "python_bridge.cc", diff --git a/source/neuropod/backends/tensorflow/BUILD b/source/neuropod/backends/tensorflow/BUILD index 615b9c78..0a9c62ac 100644 --- a/source/neuropod/backends/tensorflow/BUILD +++ b/source/neuropod/backends/tensorflow/BUILD @@ -14,8 +14,9 @@ load("@bazel_tools//tools/build_defs/pkg:pkg.bzl", "pkg_tar") load("//bazel:copy_libs.bzl", "copy_libs") +load("//bazel:cc.bzl", "neuropod_cc_binary", "neuropod_cc_library") -cc_binary( +neuropod_cc_binary( name = "libneuropod_tensorflow_backend.so", srcs = [ "//neuropod:libneuropod.so", @@ -37,7 +38,7 @@ cc_binary( ], ) -cc_library( +neuropod_cc_library( name = "tensorflow_backend", srcs = [ "tf_backend.cc", diff --git a/source/neuropod/backends/tensorflow/tf_backend.cc b/source/neuropod/backends/tensorflow/tf_backend.cc index 0bd75833..c30ad67d 100644 --- a/source/neuropod/backends/tensorflow/tf_backend.cc +++ b/source/neuropod/backends/tensorflow/tf_backend.cc @@ -186,12 +186,12 @@ void TensorflowNeuropodBackend::load_model_internal() // Create a buffer of the right size graph_stream->seekg(0, graph_stream->end); - std::streamsize graph_length = graph_stream->tellg(); + auto graph_length = static_cast(graph_stream->tellg()); std::vector buffer(graph_length); // Read into the buffer graph_stream->seekg(0, graph_stream->beg); - graph_stream->read(buffer.data(), graph_length); + graph_stream->read(buffer.data(), static_cast(graph_length)); if (graph_stream->fail()) { NEUROPOD_ERROR("Error reading TensorFlow GraphDef for neuropod {}", neuropod_path_); diff --git a/source/neuropod/backends/tensorflow/tf_tensor.cc b/source/neuropod/backends/tensorflow/tf_tensor.cc index 09008d52..e2694ecc 100644 --- a/source/neuropod/backends/tensorflow/tf_tensor.cc +++ b/source/neuropod/backends/tensorflow/tf_tensor.cc @@ -81,7 +81,7 @@ class NeuropodTensorBuffer : public tensorflow::TensorBuffer void FillAllocationDescription(tensorflow::AllocationDescription *proto) const override { - tensorflow::int64 rb = size(); + auto rb = static_cast(size()); proto->set_requested_bytes(rb); proto->set_allocator_name(tensorflow::cpu_allocator()->Name()); } @@ -116,11 +116,11 @@ tensorflow::TensorShape get_tf_shape(const std::vector &dims) // Convert shapes std::vector get_dims(const tensorflow::Tensor &tensor) { - int num_dims = tensor.dims(); + auto num_dims = static_cast(tensor.dims()); std::vector shape(num_dims); - for (int i = 0; i < num_dims; i++) + for (size_t i = 0; i < num_dims; i++) { - shape[i] = tensor.dim_size(i); + shape[i] = tensor.dim_size(static_cast(i)); } return shape; diff --git a/source/neuropod/backends/tensorflow/tf_tensor.hh b/source/neuropod/backends/tensorflow/tf_tensor.hh index 093e8df7..e0c5b693 100644 --- a/source/neuropod/backends/tensorflow/tf_tensor.hh +++ b/source/neuropod/backends/tensorflow/tf_tensor.hh @@ -160,26 +160,26 @@ public: void copy_from(const std::vector &data) { auto flat = tensor_.flat(); - if (data.size() != flat.size()) + if (data.size() != static_cast(flat.size())) { NEUROPOD_ERROR("Supplied vector size ({}) does not match size of tensor ({})", data.size(), flat.size()); } - for (int i = 0; i < data.size(); i++) + for (size_t i = 0; i < data.size(); i++) { - flat(i) = data[i]; + flat(static_cast(i)) = data[i]; } } tensorflow::Tensor &get_native_data() { return tensor_; } protected: - std::string get(size_t index) const { return tensor_.flat()(index); } + std::string get(size_t index) const { return tensor_.flat()(static_cast(index)); } void set(size_t index, const std::string &value) { - auto flat = tensor_.flat(); - flat(index) = value; + auto flat = tensor_.flat(); + flat(static_cast(index)) = value; } }; diff --git a/source/neuropod/backends/torchscript/BUILD b/source/neuropod/backends/torchscript/BUILD index c5fd6aa5..fc1973e3 100644 --- a/source/neuropod/backends/torchscript/BUILD +++ b/source/neuropod/backends/torchscript/BUILD @@ -14,8 +14,9 @@ load("@bazel_tools//tools/build_defs/pkg:pkg.bzl", "pkg_tar") load("//bazel:copy_libs.bzl", "copy_libs") +load("//bazel:cc.bzl", "neuropod_cc_binary", "neuropod_cc_library") -cc_binary( +neuropod_cc_binary( name = "libneuropod_torchscript_backend.so", srcs = [ "//neuropod:libneuropod.so", @@ -40,10 +41,11 @@ cc_binary( ], ) -cc_library( +neuropod_cc_library( name = "torch_backend", srcs = [ "torch_backend.cc", + "torch_tensor.cc", "type_utils.cc", ], hdrs = [ diff --git a/source/neuropod/backends/torchscript/torch_backend.cc b/source/neuropod/backends/torchscript/torch_backend.cc index 0f1b4e1f..9c845893 100644 --- a/source/neuropod/backends/torchscript/torch_backend.cc +++ b/source/neuropod/backends/torchscript/torch_backend.cc @@ -35,15 +35,15 @@ namespace { #if CAFFE2_NIGHTLY_VERSION >= 20200115 -#define MAKE_DICT(name, type) torch::Dict name; +#define MAKE_DICT(name, type) torch::Dict name #elif CAFFE2_NIGHTLY_VERSION >= 20191010 -#define MAKE_DICT(name, type) c10::impl::GenericDict name(c10::AnyType::get(), c10::AnyType::get()); +#define MAKE_DICT(name, type) c10::impl::GenericDict name(c10::AnyType::get(), c10::AnyType::get()) #elif CAFFE2_NIGHTLY_VERSION >= 20190717 -#define MAKE_DICT(name, type) c10::impl::GenericDict name((c10::impl::deprecatedUntypedDict())); +#define MAKE_DICT(name, type) c10::impl::GenericDict name((c10::impl::deprecatedUntypedDict())) #elif CAFFE2_NIGHTLY_VERSION >= 20190601 -#define MAKE_DICT(name, type) auto name = c10::make_dict(); +#define MAKE_DICT(name, type) auto name = c10::make_dict() #else -#define MAKE_DICT(name, type) torch::ivalue::UnorderedMap name; +#define MAKE_DICT(name, type) torch::ivalue::UnorderedMap name #endif #if CAFFE2_NIGHTLY_VERSION >= 20190717 @@ -55,11 +55,11 @@ namespace #if CAFFE2_NIGHTLY_VERSION >= 20190601 #define KEY(elem) (elem.key()) #define VALUE(elem) (elem.value()) -#define DICT_INSERT(dict, key, value) dict.insert(key, value); +#define DICT_INSERT(dict, key, value) dict.insert(key, value) #else #define KEY(elem) (elem.first) #define VALUE(elem) (elem.second) -#define DICT_INSERT(dict, key, value) dict[key] = value; +#define DICT_INSERT(dict, key, value) dict[key] = value #endif std::shared_ptr load_model_from_path(std::istream & graph_stream, @@ -133,8 +133,8 @@ void insert_value_in_output(NeuropodValueMap & output, auto tensor = value.toTensor().to(torch::kCPU); // Get the type and make a TorchNeuropodTensor - auto tensor_type = get_neuropod_type_from_torch_type(tensor.scalar_type()); - auto neuropod_tensor = make_tensor(tensor_type, tensor); + auto neuropod_tensor_type = get_neuropod_type_from_torch_type(tensor.scalar_type()); + auto neuropod_tensor = make_tensor(neuropod_tensor_type, tensor); // Add it to our output auto &to_set = output[name]; @@ -287,7 +287,7 @@ torch::Device TorchNeuropodBackend::get_torch_device(NeuropodDeviceType target_d return torch::kCPU; } - return torch::Device(torch::kCUDA, options_.visible_device); + return torch::Device(torch::kCUDA, static_cast(options_.visible_device)); } // Run inference @@ -381,7 +381,7 @@ std::unique_ptr TorchNeuropodBackend::infer_internal(const Neu schema); } - torch_inputs.at(arg_index.value() - (has_class_type ? 1 : 0)) = input_data; + torch_inputs.at(static_cast(arg_index.value() - (has_class_type ? 1 : 0))) = input_data; } } @@ -432,7 +432,7 @@ std::unique_ptr TorchNeuropodBackend::infer_internal(const Neu { // This is a named tuple // NOLINTNEXTLINE(modernize-loop-convert): Can't always use a range based loop here - for (int i = 0; i < elems.size(); i++) + for (size_t i = 0; i < elems.size(); i++) { insert_value_in_output(*to_return, GET_NAME(i), elems.at(i)); } diff --git a/source/neuropod/backends/torchscript/torch_tensor.cc b/source/neuropod/backends/torchscript/torch_tensor.cc new file mode 100644 index 00000000..b3ccae66 --- /dev/null +++ b/source/neuropod/backends/torchscript/torch_tensor.cc @@ -0,0 +1,26 @@ +/* Copyright (c) 2020 UATC, LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include "neuropod/backends/torchscript/torch_tensor.hh" + +namespace neuropod +{ + +torch::jit::IValue get_ivalue_from_torch_tensor(const std::shared_ptr &tensor) +{ + return std::dynamic_pointer_cast>(tensor)->get_native_data(); +} + +} // namespace neuropod diff --git a/source/neuropod/backends/torchscript/torch_tensor.hh b/source/neuropod/backends/torchscript/torch_tensor.hh index f798b941..d3f96d82 100644 --- a/source/neuropod/backends/torchscript/torch_tensor.hh +++ b/source/neuropod/backends/torchscript/torch_tensor.hh @@ -29,6 +29,11 @@ limitations under the License. // If we're not building with a nightly relase of torch, // set the date to match the date of the official release #ifndef CAFFE2_NIGHTLY_VERSION +#if CAFFE2_VERSION == 10100 +// The date of the official torch 1.1.0 release +#define CAFFE2_NIGHTLY_VERSION 20190430 +#endif + #if CAFFE2_VERSION == 10200 // The date of the official torch 1.2.0 release #define CAFFE2_NIGHTLY_VERSION 20190808 @@ -67,27 +72,27 @@ T *get_data_from_torch_tensor(const torch::Tensor &tensor) } template <> -uint16_t *get_data_from_torch_tensor(const torch::Tensor &tensor) +[[maybe_unused]] uint16_t *get_data_from_torch_tensor(const torch::Tensor & /*unused*/) { NEUROPOD_ERROR("TorchScript doesn't support type uint16_t"); } template <> -uint32_t *get_data_from_torch_tensor(const torch::Tensor &tensor) +[[maybe_unused]] uint32_t *get_data_from_torch_tensor(const torch::Tensor & /*unused*/) { NEUROPOD_ERROR("TorchScript doesn't support type uint32_t"); } template <> -uint64_t *get_data_from_torch_tensor(const torch::Tensor &tensor) +[[maybe_unused]] uint64_t *get_data_from_torch_tensor(const torch::Tensor & /*unused*/) { NEUROPOD_ERROR("TorchScript doesn't support type uint64_t"); } -torch::Deleter get_torch_deleter(const Deleter &deleter, void *data) +[[maybe_unused]] torch::Deleter get_torch_deleter(const Deleter &deleter, void *data) { auto handle = register_deleter(deleter, data); - return [handle](void *unused) { run_deleter(handle); }; + return [handle](void * /*unused*/) { run_deleter(handle); }; } } // namespace @@ -116,7 +121,7 @@ public: } // Wrap an existing torch tensor - TorchNeuropodTensor(torch::Tensor tensor) : TypedNeuropodTensor(tensor.sizes().vec()), tensor(tensor) {} + TorchNeuropodTensor(torch::Tensor t) : TypedNeuropodTensor(t.sizes().vec()), tensor(t) {} ~TorchNeuropodTensor() = default; @@ -147,7 +152,7 @@ protected: } else { - out = tensor.to(torch::Device(torch::kCUDA, device)); + out = tensor.to(torch::Device(torch::kCUDA, static_cast(device))); } return std::make_shared>(std::move(out)); @@ -158,7 +163,7 @@ protected: #define ELEMENTS(collection) collection #define GET_STRING_FROM_LIST(item) item #else -#define ELEMENTS(collectionptr) collectionptr->elements(); +#define ELEMENTS(collectionptr) collectionptr->elements() #define GET_STRING_FROM_LIST(item) item.toStringRef() #endif @@ -254,9 +259,6 @@ protected: }; // Utility function to get an IValue from a torch tensor -torch::jit::IValue get_ivalue_from_torch_tensor(const std::shared_ptr &tensor) -{ - return std::dynamic_pointer_cast>(tensor)->get_native_data(); -} +torch::jit::IValue get_ivalue_from_torch_tensor(const std::shared_ptr &tensor); } // namespace neuropod diff --git a/source/neuropod/internal/backend_registration.hh b/source/neuropod/internal/backend_registration.hh index 8746db7d..944e8654 100644 --- a/source/neuropod/internal/backend_registration.hh +++ b/source/neuropod/internal/backend_registration.hh @@ -75,7 +75,7 @@ BackendFactoryFunction get_backend_for_type(const std::vector & // A macro to easily define a backend // Example: REGISTER_NEUROPOD_BACKEND(SomeTensorflowBackend, "tensorflow", "1.13.1") #define REGISTER_NEUROPOD_BACKEND(CLS, type, version) \ - bool is_registered_##CLS = register_backend(#CLS, type, version, createNeuropodBackend); + static bool is_registered_##CLS = register_backend(#CLS, type, version, createNeuropodBackend); // Utility macros #define STR_HELPER(x) #x From 63939bd1c1c0cfaccf0dce6bbf9adc0c01321529 Mon Sep 17 00:00:00 2001 From: Vivek Panyam Date: Tue, 22 Sep 2020 01:09:46 -0400 Subject: [PATCH 2/2] [Build] Refactor OPE code to adhere to stricter build warnings --- source/neuropod/multiprocess/BUILD | 14 +++++++------ .../neuropod/multiprocess/control_messages.cc | 2 +- source/neuropod/multiprocess/mq/BUILD | 3 ++- .../multiprocess/mq/ipc_message_queue.hh | 2 -- .../multiprocess/mq/ipc_message_queue_impl.hh | 2 +- .../multiprocess/mq/wire_format_impl.hh | 8 ++++---- source/neuropod/multiprocess/multiprocess.cc | 7 ++++--- .../multiprocess/multiprocess_worker.cc | 2 ++ .../neuropod/multiprocess/serialization/BUILD | 3 ++- .../serialization/ipc_serialization.hh | 16 +++++++-------- source/neuropod/multiprocess/shm/BUILD | 3 ++- .../shm/raw_shm_block_allocator.cc | 9 +++++---- .../multiprocess/shm/shm_allocator.cc | 7 +++---- source/neuropod/multiprocess/shm_tensor.hh | 20 +++++++++---------- source/neuropod/multiprocess/tensor_utils.hh | 8 ++++---- 15 files changed, 56 insertions(+), 50 deletions(-) diff --git a/source/neuropod/multiprocess/BUILD b/source/neuropod/multiprocess/BUILD index 73bd382e..fc072539 100644 --- a/source/neuropod/multiprocess/BUILD +++ b/source/neuropod/multiprocess/BUILD @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -cc_library( +load("//bazel:cc.bzl", "neuropod_cc_binary", "neuropod_cc_library") + +neuropod_cc_library( name = "shm_tensor", srcs = [ "shm_tensor.cc", @@ -29,7 +31,7 @@ cc_library( ], ) -cc_library( +neuropod_cc_library( name = "ipc_control_channel", srcs = [ "control_messages.cc", @@ -49,7 +51,7 @@ cc_library( ], ) -cc_library( +neuropod_cc_library( name = "multiprocess_worker", srcs = [ "multiprocess_worker.cc", @@ -68,7 +70,7 @@ cc_library( ], ) -cc_binary( +neuropod_cc_binary( name = "neuropod_multiprocess_worker", srcs = [ "multiprocess_worker_main.cc", @@ -87,7 +89,7 @@ cc_binary( ], ) -cc_library( +neuropod_cc_library( name = "multiprocess", hdrs = [ "multiprocess.hh", @@ -103,7 +105,7 @@ cc_library( ], ) -cc_library( +neuropod_cc_library( name = "impl", srcs = [ "multiprocess.cc", diff --git a/source/neuropod/multiprocess/control_messages.cc b/source/neuropod/multiprocess/control_messages.cc index 9565dbbf..7187aa7e 100644 --- a/source/neuropod/multiprocess/control_messages.cc +++ b/source/neuropod/multiprocess/control_messages.cc @@ -25,7 +25,7 @@ std::ostream &operator<<(std::ostream &out, const MessageType value) #define GENERATE_CASE(item) \ case (item): \ s = #item; \ - break; + break switch (value) { GENERATE_CASE(LOAD_NEUROPOD); diff --git a/source/neuropod/multiprocess/mq/BUILD b/source/neuropod/multiprocess/mq/BUILD index ff8813e7..3bff9be9 100644 --- a/source/neuropod/multiprocess/mq/BUILD +++ b/source/neuropod/multiprocess/mq/BUILD @@ -11,8 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +load("//bazel:cc.bzl", "neuropod_cc_library") -cc_library( +neuropod_cc_library( name = "mq", srcs = [ "ipc_message_queue.cc", diff --git a/source/neuropod/multiprocess/mq/ipc_message_queue.hh b/source/neuropod/multiprocess/mq/ipc_message_queue.hh index 81354239..cb35c3b9 100644 --- a/source/neuropod/multiprocess/mq/ipc_message_queue.hh +++ b/source/neuropod/multiprocess/mq/ipc_message_queue.hh @@ -55,8 +55,6 @@ private: QueueMessage(std::shared_ptr> data) : data_(std::move(data)) {} public: - ~QueueMessage() = default; - // Get a payload of type `Payload` from this message template void get(Payload &out) diff --git a/source/neuropod/multiprocess/mq/ipc_message_queue_impl.hh b/source/neuropod/multiprocess/mq/ipc_message_queue_impl.hh index c2870142..19378e64 100644 --- a/source/neuropod/multiprocess/mq/ipc_message_queue_impl.hh +++ b/source/neuropod/multiprocess/mq/ipc_message_queue_impl.hh @@ -29,7 +29,7 @@ namespace detail extern std::atomic_uint64_t msg_counter; // The max size for the send and recv control queues -constexpr auto MAX_QUEUE_SIZE = 20; +constexpr size_t MAX_QUEUE_SIZE = 20; template inline std::unique_ptr make_queue(const std::string &control_queue_name_, const std::string &suffix) diff --git a/source/neuropod/multiprocess/mq/wire_format_impl.hh b/source/neuropod/multiprocess/mq/wire_format_impl.hh index 48e06644..168b816f 100644 --- a/source/neuropod/multiprocess/mq/wire_format_impl.hh +++ b/source/neuropod/multiprocess/mq/wire_format_impl.hh @@ -108,13 +108,13 @@ void serialize_payload(const Payload &payload, WireFormat &data ipc_serialize(ss, payload); // Set the size - auto size_bytes = ss.tellp(); - data.payload_size = size_bytes; + auto size_bytes = static_cast(ss.tellp()); + data.payload_size = static_cast(size_bytes); if (size_bytes <= sizeof(data.payload)) { // We can store this message inline - ss.read(data.payload, size_bytes); + ss.read(data.payload, static_cast(size_bytes)); data.is_inline = true; } else @@ -126,7 +126,7 @@ void serialize_payload(const Payload &payload, WireFormat &data auto block = shm_allocator.allocate_shm(size_bytes, block_id); // Write the serialized message into the block - ss.read(static_cast(block.get()), size_bytes); + ss.read(static_cast(block.get()), static_cast(size_bytes)); // Copy the block id into the message memcpy(data.payload_id, block_id.data(), sizeof(data.payload_id)); diff --git a/source/neuropod/multiprocess/multiprocess.cc b/source/neuropod/multiprocess/multiprocess.cc index 0377adc6..4a1b0edb 100644 --- a/source/neuropod/multiprocess/multiprocess.cc +++ b/source/neuropod/multiprocess/multiprocess.cc @@ -76,17 +76,18 @@ pid_t start_worker_process(const std::string &control_queue_name, std::vector env_arr(env.size() + 1); env_arr[env.size()] = nullptr; // Set the env - for (int i = 0; i < env.size(); i++) + for (size_t i = 0; i < env.size(); i++) { env_arr[i] = const_cast(env[i].c_str()); } // Spawn a process - const auto status = posix_spawnp(&child_pid, "neuropod_multiprocess_worker", nullptr, nullptr, argv, env_arr); + const auto status = + posix_spawnp(&child_pid, "neuropod_multiprocess_worker", nullptr, nullptr, argv, env_arr.data()); if (status != 0) { NEUROPOD_ERROR("Failed to start the worker process. Failed with code: {} - {}", status, strerror(status)); diff --git a/source/neuropod/multiprocess/multiprocess_worker.cc b/source/neuropod/multiprocess/multiprocess_worker.cc index eed3fff5..e3c9c78c 100644 --- a/source/neuropod/multiprocess/multiprocess_worker.cc +++ b/source/neuropod/multiprocess/multiprocess_worker.cc @@ -13,6 +13,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +#include "neuropod/multiprocess/multiprocess_worker.hh" + #include "neuropod/internal/logging.hh" #include "neuropod/multiprocess/control_messages.hh" #include "neuropod/multiprocess/ipc_control_channel.hh" diff --git a/source/neuropod/multiprocess/serialization/BUILD b/source/neuropod/multiprocess/serialization/BUILD index dbc78447..3ce0b23f 100644 --- a/source/neuropod/multiprocess/serialization/BUILD +++ b/source/neuropod/multiprocess/serialization/BUILD @@ -11,8 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +load("//bazel:cc.bzl", "neuropod_cc_library") -cc_library( +neuropod_cc_library( name = "serialization", hdrs = [ "ipc_serialization.hh", diff --git a/source/neuropod/multiprocess/serialization/ipc_serialization.hh b/source/neuropod/multiprocess/serialization/ipc_serialization.hh index 96243deb..3e66f0af 100644 --- a/source/neuropod/multiprocess/serialization/ipc_serialization.hh +++ b/source/neuropod/multiprocess/serialization/ipc_serialization.hh @@ -33,20 +33,20 @@ namespace detail { // Utilities for checked reads and writes to and from streams -template -inline void checked_write(std::ostream &stream, Params &&... params) +template +inline void checked_write(std::ostream &stream, T &&t, size_t len) { - stream.write(std::forward(params)...); + stream.write(std::forward(t), static_cast(len)); if (!stream) { NEUROPOD_ERROR("Writing to stream failed during IPC serialization"); } } -template -inline void checked_read(std::istream &stream, Params &&... params) +template +inline void checked_read(std::istream &stream, T &&t, size_t len) { - stream.read(std::forward(params)...); + stream.read(std::forward(t), static_cast(len)); if (!stream) { NEUROPOD_ERROR("Reading from stream failed during IPC serialization"); @@ -190,7 +190,7 @@ inline void ipc_deserialize(std::istream &in, std::vector &item) ipc_deserialize(in, size); // Get the content - for (int i = 0; i < size; i++) + for (size_t i = 0; i < size; i++) { T elem; ipc_deserialize(in, elem); @@ -223,7 +223,7 @@ inline void ipc_deserialize(std::istream &in, std::unordered_map &item) size_t num_items; ipc_deserialize(in, num_items); - for (int i = 0; i < num_items; i++) + for (size_t i = 0; i < num_items; i++) { // Read the key K key; diff --git a/source/neuropod/multiprocess/shm/BUILD b/source/neuropod/multiprocess/shm/BUILD index eb415cbd..8cad38a6 100644 --- a/source/neuropod/multiprocess/shm/BUILD +++ b/source/neuropod/multiprocess/shm/BUILD @@ -11,8 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +load("//bazel:cc.bzl", "neuropod_cc_library") -cc_library( +neuropod_cc_library( name = "shm", srcs = [ "raw_shm_block_allocator.cc", diff --git a/source/neuropod/multiprocess/shm/raw_shm_block_allocator.cc b/source/neuropod/multiprocess/shm/raw_shm_block_allocator.cc index 11ae6f84..7da1d859 100644 --- a/source/neuropod/multiprocess/shm/raw_shm_block_allocator.cc +++ b/source/neuropod/multiprocess/shm/raw_shm_block_allocator.cc @@ -69,7 +69,8 @@ thread_local boost::uuids::random_generator uuid_generator; // A unique handle for a Raw SHM block // This is currently just a UUID -struct __attribute__((__packed__)) RawSHMHandleInternal +// Note: Make sure to add `__attribute__((__packed__))` if adding more fields to this struct +struct RawSHMHandleInternal { boost::uuids::uuid uuid; }; @@ -102,7 +103,7 @@ class RawSHMBlock ipc::create_only, get_key_from_uuid(uuid_).c_str(), ipc::read_write); // Set the size - shm_->truncate(sizeof(RawSHMBlockInternal) + size_bytes); + shm_->truncate(static_cast(sizeof(RawSHMBlockInternal) + size_bytes)); // Map into memory region_ = stdx::make_unique(*shm_, ipc::read_write); @@ -210,7 +211,7 @@ std::shared_ptr RawSHMBlockAllocator::allocate_shm(size_t size_bytes, RawS // Create a shared pointer to the underlying data with a custom deleter // that keeps the block alive - return std::shared_ptr(block->get_data(), [block](void *unused) {}); + return std::shared_ptr(block->get_data(), [block](void * /*unused*/) {}); } std::shared_ptr RawSHMBlockAllocator::load_shm(const RawSHMHandle &handle) @@ -220,7 +221,7 @@ std::shared_ptr RawSHMBlockAllocator::load_shm(const RawSHMHandle &handle) // Create a shared pointer to the underlying data with a custom deleter // that keeps the block alive - return std::shared_ptr(block->get_data(), [block](void *unused) {}); + return std::shared_ptr(block->get_data(), [block](void * /*unused*/) {}); } } // namespace neuropod diff --git a/source/neuropod/multiprocess/shm/shm_allocator.cc b/source/neuropod/multiprocess/shm/shm_allocator.cc index 3b71b77b..b8bf2661 100644 --- a/source/neuropod/multiprocess/shm/shm_allocator.cc +++ b/source/neuropod/multiprocess/shm/shm_allocator.cc @@ -53,7 +53,6 @@ struct __attribute__((__packed__)) SHMBlockInternal // Delete the copy and move constructors // (if the copy constructors are deleted, no move constructors will be autogenerated) SHMBlockInternal() = default; - ~SHMBlockInternal() = default; SHMBlockInternal(const SHMBlockInternal &) = delete; SHMBlockInternal &operator=(const SHMBlockInternal &) = delete; }; @@ -152,7 +151,7 @@ class AllocationCache auto &cache_item = *it; auto *cached_block = static_cast(cache_item.block.get()); - ipc::scoped_lock lock(cached_block->mutex); + ipc::scoped_lock block_lock(cached_block->mutex); // Check whether or not this block is used in another process if (cached_block->refcount == 0) @@ -231,7 +230,7 @@ std::shared_ptr SHMAllocator::allocate_shm(size_t size_bytes, SHMBlockID & // Create a shared pointer to the underlying data with a custom deleter // that keeps the block alive. Add the block to the cache on destruction. return std::shared_ptr( - block->data, [this, block, raw_block = std::move(raw_block), requested_size, id](void *unused) mutable { + block->data, [this, block, raw_block = std::move(raw_block), requested_size, id](void * /*unused*/) mutable { { // Lock the block's mutex ipc::scoped_lock lock(block->mutex); @@ -302,7 +301,7 @@ std::shared_ptr SHMAllocator::load_shm(const SHMBlockID &block_id) // Create a shared pointer to the underlying data with a custom deleter // that keeps the block alive. Add the block to the cache on destruction. return std::shared_ptr(block->data, - [this, block, raw_block = std::move(raw_block), handle](void *unused) mutable { + [this, block, raw_block = std::move(raw_block), handle](void * /*unused*/) mutable { { // Lock the block's mutex ipc::scoped_lock lock(block->mutex); diff --git a/source/neuropod/multiprocess/shm_tensor.hh b/source/neuropod/multiprocess/shm_tensor.hh index 2f680566..c5568a16 100644 --- a/source/neuropod/multiprocess/shm_tensor.hh +++ b/source/neuropod/multiprocess/shm_tensor.hh @@ -134,7 +134,7 @@ public: const void *get_untyped_data_ptr() const { return data_->data; } - SHMBlockID get_native_data() { return block_id_; }; + SHMBlockID get_native_data() { return block_id_; } }; std::shared_ptr tensor_from_id(const SHMBlockID &block_id); @@ -186,20 +186,20 @@ public: // Load an existing tensor // See tensor_from_id - SHMNeuropodTensor(const std::vector &dims, - std::shared_ptr block, - shm_tensor * data, - const SHMBlockID & block_id) + [[maybe_unused]] SHMNeuropodTensor(const std::vector &dims, + std::shared_ptr block, + shm_tensor * data, + const SHMBlockID & block_id) : TypedNeuropodTensor(copy_and_strip_last_dim(dims)), write_buffer_(this->get_num_elements()) { auto byte_block = stdx::make_unique>(dims, std::move(block), data, block_id); auto base_ptr = byte_block->get_raw_data_ptr(); - auto max_len = dims[dims.size() - 1]; + auto max_len = static_cast(dims[dims.size() - 1]); // Copy into our local buffer auto numel = get_num_elements(); - for (int i = 0; i < numel; i++) + for (size_t i = 0; i < numel; i++) { auto pos = base_ptr + i * max_len; auto wrapper = reinterpret_cast(pos); @@ -226,7 +226,7 @@ public: // Add it to dims auto dims_copy = get_dims(); - dims_copy.push_back(max_len); + dims_copy.push_back(static_cast(max_len)); // TODO(vip): We can optimize this last_shm_block_ = stdx::make_unique>(dims_copy); @@ -239,7 +239,7 @@ public: auto wrapper = reinterpret_cast(pos); // Set the item size - wrapper->length = item.size(); + wrapper->length = static_cast(item.size()); // Copy in the data std::copy(item.begin(), item.end(), wrapper->data); @@ -249,7 +249,7 @@ public: } return last_shm_block_->get_native_data(); - }; + } protected: std::string get(size_t index) const { return write_buffer_.at(index); } diff --git a/source/neuropod/multiprocess/tensor_utils.hh b/source/neuropod/multiprocess/tensor_utils.hh index 42889e36..5bfb39a1 100644 --- a/source/neuropod/multiprocess/tensor_utils.hh +++ b/source/neuropod/multiprocess/tensor_utils.hh @@ -27,8 +27,8 @@ namespace neuropod // This is useful for serialization and to wrap and/or copy tensors between backends. // For example, if you had a TorchNeuropodTensor and you wanted to get a tensor compatible // with `allocator` without making a copy, you could use this function -std::shared_ptr wrap_existing_tensor(NeuropodTensorAllocator & allocator, - std::shared_ptr tensor) +static std::shared_ptr wrap_existing_tensor(NeuropodTensorAllocator & allocator, + std::shared_ptr tensor) { // Whenever you're wrapping existing memory, it is very important to make sure that the data // being wrapped does not get deleted before the underlying DL framework is done with the @@ -36,7 +36,7 @@ std::shared_ptr wrap_existing_tensor(NeuropodTensorAllocator & // // In this case, we're capturing `tensor` in the deleter below. This ensures that the tensor // doesn't get deallocated until we're done with the new tensor. - const auto deleter = [tensor](void *unused) {}; + const auto deleter = [tensor](void * /*unused*/) {}; const auto &tensor_type = tensor->get_tensor_type(); if (tensor_type == STRING_TENSOR) { @@ -74,7 +74,7 @@ std::shared_ptr wrap_existing_tensor(std::shared_ptrget_tensor_type(); if (tensor_type == STRING_TENSOR) {