Skip to content

Commit

Permalink
[OPE] Improve IPC serialization of structs (#362)
Browse files Browse the repository at this point in the history
This PR uses reflection to serialize structs instead of requiring us to define `ipc_serialize` and `ipc_deserialize` for every struct we want to pass to the worker process.

This is much less error prone than the manual method since adding a field to a struct no longer requires modifying serialization code.

As part of this, we switch to building Neuropod with C++17. We still build in a manylinux2014 compatible sysroot so we can't use all C++17 library features, but this lets us use some C++17 language features.
  • Loading branch information
VivekPanyam authored May 28, 2020
1 parent 6f9badc commit e2afeab
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 68 deletions.
5 changes: 3 additions & 2 deletions source/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ build --test_env=VIRTUAL_ENV
# Use an internal toolchain to isolate the build from system libraries
build --crosstool_top=@llvm_toolchain//:toolchain

# Use C++14
build --cxxopt='-std=c++14'
# Use some C++17 features. We still build in a C++14 sysroot so can't use all of them
# (Linking will fail during linux builds if you're using a C++17 feature that isn't available)
build --cxxopt='-std=c++17'

# Build in a mostly-static way
build --dynamic_mode off
Expand Down
8 changes: 8 additions & 0 deletions source/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,11 @@ http_archive(
strip_prefix = "cpp-semver-ff341259099465283bc3dc60fb5ede16776e1607",
url = "https://github.com/easz/cpp-semver/archive/ff341259099465283bc3dc60fb5ede16776e1607.zip",
)

http_archive(
name = "pfr_repo",
build_file = "@//deps:BUILD.pfr",
sha256 = "92e0ffa3be49ffb21486e8e929dde599455fc1818054254bac832bcbc2c5491b",
strip_prefix = "magic_get-1.0.0",
url = "https://github.com/apolukhin/magic_get/archive/1.0.0.zip",
)
28 changes: 28 additions & 0 deletions source/deps/BUILD.pfr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# 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.

package(
default_visibility = ["//visibility:public"],
)

cc_library(
name = "pfr",
hdrs = glob([
"include/boost/**",
]),
includes = [
"include"
],
visibility = ["//visibility:public"],
)
17 changes: 15 additions & 2 deletions source/neuropod/backends/tensorflow/tf_tensor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ limitations under the License.
#include "neuropod/internal/error_utils.hh"
#include "neuropod/internal/logging.hh"
#include "neuropod/internal/neuropod_tensor.hh"
#include "tensorflow/core/common_runtime/dma_helper.h"
#include "tensorflow/core/framework/tensor.h"
#include "tensorflow/core/public/version.h"

Expand Down Expand Up @@ -104,14 +105,26 @@ protected:
void *get_untyped_data_ptr()
{
// TODO(vip): make sure this tensor is contiguous
return const_cast<char *>(tensor_.tensor_data().data());
auto buf = tensorflow::DMAHelper::buffer(&tensor_);
if (buf != nullptr)
{
return buf->data();
}

return nullptr;
}

// Get a pointer to the underlying data
const void *get_untyped_data_ptr() const
{
// TODO(vip): make sure this tensor is contiguous
return tensor_.tensor_data().data();
auto buf = tensorflow::DMAHelper::buffer(&tensor_);
if (buf != nullptr)
{
return buf->data();
}

return nullptr;
}
};

Expand Down
1 change: 0 additions & 1 deletion source/neuropod/multiprocess/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ cc_library(
srcs = [
"control_messages.cc",
"ipc_control_channel.cc",
"ope_load_config.cc",
],
hdrs = [
"control_messages.hh",
Expand Down
52 changes: 0 additions & 52 deletions source/neuropod/multiprocess/ope_load_config.cc

This file was deleted.

7 changes: 0 additions & 7 deletions source/neuropod/multiprocess/ope_load_config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,4 @@ struct ope_load_config
std::vector<BackendLoadSpec> default_backend_overrides;
};

// Serialization specializations for ope_load_config
template <>
void ipc_serialize(std::ostream &out, const ope_load_config &data);

template <>
void ipc_deserialize(std::istream &in, ope_load_config &data);

} // namespace neuropod
1 change: 1 addition & 0 deletions source/neuropod/multiprocess/serialization/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ cc_library(
deps = [
"//neuropod/backends:neuropod_backend",
"@boost_repo//:boost",
"@pfr_repo//:pfr",
],
)
34 changes: 30 additions & 4 deletions source/neuropod/multiprocess/serialization/ipc_serialization.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ limitations under the License.
#include "neuropod/internal/error_utils.hh"
#include "neuropod/internal/memory_utils.hh"

#include <boost/pfr/precise.hpp>

#include <istream>
#include <ostream>
#include <string>
Expand Down Expand Up @@ -62,18 +64,42 @@ inline void checked_read(std::istream &stream, Params &&... params)
template <typename T>
inline void ipc_serialize(std::ostream &out, const T &item)
{
static_assert(std::is_integral<T>::value, "The ipc_serialize function must be specialized for the requested type");
constexpr bool is_integral = std::is_integral<T>::value;
constexpr bool is_aggregate = std::is_aggregate<T>::value;

static_assert(is_integral || is_aggregate, "The ipc_serialize function must be specialized for the requested type");

detail::checked_write(out, reinterpret_cast<const char *>(&item), sizeof(item));
if constexpr (is_integral)
{
// Primitive types
detail::checked_write(out, reinterpret_cast<const char *>(&item), sizeof(item));
}
else if (is_aggregate)
{
// Structs
boost::pfr::for_each_field(item, [&](auto &field) { ipc_serialize(out, field); });
}
}

template <typename T>
inline void ipc_deserialize(std::istream &in, T &item)
{
static_assert(std::is_integral<T>::value,
constexpr bool is_integral = std::is_integral<T>::value;
constexpr bool is_aggregate = std::is_aggregate<T>::value;

static_assert(is_integral || is_aggregate,
"The ipc_deserialize function must be specialized for the requested type");

detail::checked_read(in, reinterpret_cast<char *>(&item), sizeof(item));
if constexpr (is_integral)
{
// Primitive types
detail::checked_read(in, reinterpret_cast<char *>(&item), sizeof(item));
}
else if (is_aggregate)
{
// Structs
boost::pfr::for_each_field(item, [&](auto &field) { ipc_deserialize(in, field); });
}
}

// Specialization for bool
Expand Down

0 comments on commit e2afeab

Please sign in to comment.