Add libnop dependency and use for tensor descriptor

Summary: Include libnop as a dependency but, for now, keep protobuf too. Provide a set of helpers for libnop, primarily to perform type erasure. In order to build and validate these helpers, use them to (de)serialize the tensor descriptors produced/consumed by channels, as they are a simple and self-contained application. Wider migration from protobuf to libnop will come later.

Reviewed By: heiner

Differential Revision: D22763734

fbshipit-source-id: 75bf8211ba9f89a3ee62e518b9ac44017a4f707e

Author: lw

.gitmodules

@@ -8,3 +8,6 @@
 [submodule "third_party/googletest"]
 	path = third_party/googletest
 	url = https://github.com/google/googletest.git
+[submodule "third_party/libnop"]
+	path = third_party/libnop
+	url = https://github.com/google/libnop.git

tensorpipe/CMakeLists.txt

@@ -43,16 +43,14 @@ target_sources(tensorpipe PRIVATE
 
 target_sources(tensorpipe PRIVATE
   channel/xth/channel.cc
-  channel/xth/context.cc
-  proto/channel/xth.proto)
+  channel/xth/context.cc)
 
 ### cma
 
 if(TP_ENABLE_CMA)
   target_sources(tensorpipe PRIVATE
     channel/cma/channel.cc
-    channel/cma/context.cc
-    proto/channel/cma.proto)
+    channel/cma/context.cc)
   set(TENSORPIPE_HAS_CMA_CHANNEL 1)
 else()
   set(TENSORPIPE_HAS_CMA_CHANNEL 0)
@@ -131,6 +129,12 @@ endif()
 configure_file(tensorpipe.h.in tensorpipe.h)
 
 
+## Libnop
+
+# Keep libnop headers private as they should not be exposed to downstream users.
+target_include_directories(tensorpipe PRIVATE ${PROJECT_SOURCE_DIR}/third_party/libnop/include)
+
+
 ## Python bindings
 
 if(TP_BUILD_PYTHON)

tensorpipe/channel/cma/channel.cc

@@ -17,6 +17,9 @@
 #include <list>
 #include <mutex>
 
+#include <nop/serializer.h>
+#include <nop/structure.h>
+
 #include <tensorpipe/channel/error.h>
 #include <tensorpipe/channel/helpers.h>
 #include <tensorpipe/common/callback.h>
@@ -26,12 +29,21 @@
 #include <tensorpipe/common/optional.h>
 #include <tensorpipe/common/queue.h>
 #include <tensorpipe/common/system.h>
-#include <tensorpipe/proto/channel/cma.pb.h>
 
 namespace tensorpipe {
 namespace channel {
 namespace cma {
 
+namespace {
+
+struct Descriptor {
+  uint32_t pid;
+  uint64_t ptr;
+  NOP_STRUCTURE(Descriptor, pid, ptr);
+};
+
+} // namespace
+
 class Channel::Impl : public std::enable_shared_from_this<Channel::Impl> {
  public:
   Impl(
@@ -221,11 +233,12 @@ void Channel::Impl::sendFromLoop_(
             callback(impl.error_);
           }));
 
-  proto::Descriptor pbDescriptor;
-  pbDescriptor.set_pid(getpid());
-  pbDescriptor.set_ptr(reinterpret_cast<uint64_t>(ptr));
+  NopHolder<Descriptor> nopHolder;
+  Descriptor& nopDescriptor = nopHolder.getObject();
+  nopDescriptor.pid = getpid();
+  nopDescriptor.ptr = reinterpret_cast<uint64_t>(ptr);
 
-  descriptorCallback(Error::kSuccess, saveDescriptor(pbDescriptor));
+  descriptorCallback(Error::kSuccess, saveDescriptor(nopHolder));
 }
 
 // Receive memory region from peer.
@@ -277,10 +290,11 @@ void Channel::Impl::recvFromLoop_(
     return;
   }
 
-  proto::Descriptor pbDescriptor;
-  loadDescriptor(pbDescriptor, descriptor);
-  pid_t remotePid = pbDescriptor.pid();
-  void* remotePtr = reinterpret_cast<void*>(pbDescriptor.ptr());
+  NopHolder<Descriptor> nopHolder;
+  loadDescriptor(nopHolder, descriptor);
+  Descriptor& nopDescriptor = nopHolder.getObject();
+  pid_t remotePid = nopDescriptor.pid;
+  void* remotePtr = reinterpret_cast<void*>(nopDescriptor.ptr);
 
   TP_VLOG(6) << "Channel " << id_ << " is copying payload (#" << sequenceNumber
              << ")";

tensorpipe/channel/helpers.cc

@@ -9,22 +9,30 @@
 #include <tensorpipe/channel/helpers.h>
 
 #include <tensorpipe/common/defs.h>
+#include <tensorpipe/common/nop.h>
 
 namespace tensorpipe {
 namespace channel {
 
-Channel::TDescriptor saveDescriptor(const google::protobuf::MessageLite& pb) {
-  Channel::TDescriptor out;
-  const auto success = pb.SerializeToString(&out);
-  TP_DCHECK(success) << "Failed to serialize protobuf message";
+Channel::TDescriptor saveDescriptor(const AbstractNopHolder& object) {
+  const size_t len = object.getSize();
+  Channel::TDescriptor out(len, '\0');
+  nop::BufferWriter writer(reinterpret_cast<uint8_t*>(out.data()), len);
+
+  nop::Status<void> status = object.write(writer);
+  TP_THROW_ASSERT_IF(status.has_error())
+      << "Error saving descriptor: " << status.GetErrorMessage();
+
   return out;
 }
 
-void loadDescriptor(
-    google::protobuf::MessageLite& pb,
-    const Channel::TDescriptor& in) {
-  const auto success = pb.ParseFromString(in);
-  TP_DCHECK(success) << "Failed to parse protobuf message";
+void loadDescriptor(AbstractNopHolder& object, const Channel::TDescriptor& in) {
+  const size_t len = in.size();
+  nop::BufferReader reader(reinterpret_cast<const uint8_t*>(in.data()), len);
+
+  nop::Status<void> status = object.read(reader);
+  TP_THROW_ASSERT_IF(status.has_error())
+      << "Error loading descriptor: " << status.GetErrorMessage();
 }
 
 } // namespace channel

tensorpipe/channel/helpers.h

@@ -10,18 +10,15 @@
 
 // Note: never include this file from headers!
 
-#include <google/protobuf/message_lite.h>
-
 #include <tensorpipe/channel/channel.h>
+#include <tensorpipe/common/nop.h>
 
 namespace tensorpipe {
 namespace channel {
 
-Channel::TDescriptor saveDescriptor(const google::protobuf::MessageLite& pb);
+Channel::TDescriptor saveDescriptor(const AbstractNopHolder& object);
 
-void loadDescriptor(
-    google::protobuf::MessageLite& pb,
-    const Channel::TDescriptor& in);
+void loadDescriptor(AbstractNopHolder& object, const Channel::TDescriptor& in);
 
 } // namespace channel
 } // namespace tensorpipe

tensorpipe/channel/xth/channel.cc

@@ -8,18 +8,29 @@
 
 #include <tensorpipe/channel/xth/channel.h>
 
+#include <nop/serializer.h>
+#include <nop/structure.h>
+
 #include <tensorpipe/channel/error.h>
 #include <tensorpipe/channel/helpers.h>
 #include <tensorpipe/common/callback.h>
 #include <tensorpipe/common/defs.h>
 #include <tensorpipe/common/error.h>
 #include <tensorpipe/common/error_macros.h>
-#include <tensorpipe/proto/channel/xth.pb.h>
 
 namespace tensorpipe {
 namespace channel {
 namespace xth {
 
+namespace {
+
+struct Descriptor {
+  uint64_t ptr;
+  NOP_STRUCTURE(Descriptor, ptr);
+};
+
+} // namespace
+
 class Channel::Impl : public std::enable_shared_from_this<Channel::Impl> {
  public:
   Impl(
@@ -208,10 +219,11 @@ void Channel::Impl::sendFromLoop_(
             callback(impl.error_);
           }));
 
-  proto::Descriptor pbDescriptor;
-  pbDescriptor.set_ptr(reinterpret_cast<std::uintptr_t>(ptr));
+  NopHolder<Descriptor> nopHolder;
+  Descriptor& nopDescriptor = nopHolder.getObject();
+  nopDescriptor.ptr = reinterpret_cast<std::uintptr_t>(ptr);
 
-  descriptorCallback(Error::kSuccess, saveDescriptor(pbDescriptor));
+  descriptorCallback(Error::kSuccess, saveDescriptor(nopHolder));
 }
 
 // Receive memory region from peer.
@@ -262,9 +274,10 @@ void Channel::Impl::recvFromLoop_(
     return;
   }
 
-  proto::Descriptor pbDescriptor;
-  loadDescriptor(pbDescriptor, descriptor);
-  void* remotePtr = reinterpret_cast<void*>(pbDescriptor.ptr());
+  NopHolder<Descriptor> nopHolder;
+  loadDescriptor(nopHolder, descriptor);
+  Descriptor& nopDescriptor = nopHolder.getObject();
+  void* remotePtr = reinterpret_cast<void*>(nopDescriptor.ptr);
   TP_VLOG(6) << "Channel " << id_ << " is copying payload (#" << sequenceNumber
              << ")";
   context_->requestCopy(

tensorpipe/common/nop.h

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <nop/serializer.h>
+#include <nop/status.h>
+#include <nop/utility/buffer_reader.h>
+#include <nop/utility/buffer_writer.h>
+
+namespace tensorpipe {
+
+// Libnop makes heavy use of templates, whereas TensorPipe is designed around
+// polymorphism (abstract interfaces and concrete derived classes). The two
+// don't mix well: for example, one can't have virtual method templates. One
+// technique to get around this is type erasure, which is however tricky to get
+// right because the "fundamental" operation(s) of libnop, (de)serialization,
+// are simultaneously templated on two types: the reader/writer and the object.
+// Ideally we'd like for both these sets of types to be dynamically extensible,
+// as we want to allow transpors to provide their own specialized readers and
+// writers, and channels could have their own custom objects that they want to
+// (de)serialize. New transports and channel could be implemented by third
+// parties and plugged in at runtime, so the sets of reader/writers and of
+// objects that we must support can't be known in advance.
+
+// We had originally found a solution to this pickle by doing two type erasures
+// one after the other, first on the reader/writer, which deals with bytes and
+// not objects and is thus not templated, and then on objects, leveraging the
+// fact that there is one libnop (de)serializer that takes a *pointer* to a
+// reader/writer giving us a "hook" on which to do polymorphism, by hardcoding a
+// pointer to the base reader/writer class as template parameter, but then
+// passing in an instance of a concrete subclass at runtime.
+
+// However it turned out that this performed poorly, apparently due to the
+// (de)serialization process consisting in many small calls to the reader/writer
+// which each had to perform a vtable lookup. So, instead, we decided to not
+// allow transports to utilize custom specialized readers/writers and to provide
+// a single global reader/writer class that is able to cover the two main usage
+// patterns we think are most likely to come up: reading/writing to a temporary
+// contiguous buffer, and reading/writing to a ringbuffer.
+
+// The helpers to perform type erasure of the object type: a untemplated base
+// class exposing the methods we need for (de)serialization, and then templated
+// subclasses allowing to create a holder for each concrete libnop type.
+
+class AbstractNopHolder {
+ public:
+  virtual size_t getSize() const = 0;
+  virtual nop::Status<void> write(nop::BufferWriter& writer) const = 0;
+  virtual nop::Status<void> read(nop::BufferReader& reader) = 0;
+  virtual ~AbstractNopHolder() = default;
+};
+
+template <typename T>
+class NopHolder : public AbstractNopHolder {
+ public:
+  T& getObject() {
+    return object_;
+  }
+
+  const T& getObject() const {
+    return object_;
+  }
+
+  size_t getSize() const override {
+    return nop::Encoding<T>::Size(object_);
+  }
+
+  nop::Status<void> write(nop::BufferWriter& writer) const override {
+    return nop::Encoding<T>::Write(object_, &writer);
+  }
+
+  nop::Status<void> read(nop::BufferReader& reader) override {
+    return nop::Encoding<T>::Read(&object_, &reader);
+  }
+
+ private:
+  T object_;
+};
+
+} // namespace tensorpipe