simplify BufVec (for now)

Summary:
Avoid some overhead and complexity by storing BufVec as a
unique_ptr<IOBuf>. The complexity can be reintroduced if we ever find
FUSE splice support to be a performance win for us.

Reviewed By: kmancini

Differential Revision: D22710795

fbshipit-source-id: e58eedc0fb5cea9e9743ccd20d3e4e2b7cc5d198

eden/fs/fuse/BufVec.cpp

@@ -1,53 +0,0 @@
-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This software may be used and distributed according to the terms of the
- * GNU General Public License version 2.
- */
-
-#include "eden/fs/fuse/BufVec.h"
-
-namespace facebook {
-namespace eden {
-
-BufVec::Buf::Buf(std::unique_ptr<folly::IOBuf> buf) : buf(std::move(buf)) {}
-
-BufVec::BufVec(std::unique_ptr<folly::IOBuf> buf) {
-  items_.emplace_back(std::make_shared<Buf>(std::move(buf)));
-}
-
-folly::fbvector<struct iovec> BufVec::getIov() const {
-  folly::fbvector<struct iovec> vec;
-
-  for (const auto& b : items_) {
-    DCHECK(b->fd == -1) << "we don't support splicing yet";
-    b->buf->appendToIov(&vec);
-  }
-
-  return vec;
-}
-
-size_t BufVec::size() const {
-  size_t total = 0;
-  for (const auto& b : items_) {
-    total += b->buf->computeChainDataLength();
-  }
-  return total;
-}
-
-std::string BufVec::copyData() const {
-  std::string rv;
-  rv.reserve(size());
-  for (const auto& b : items_) {
-    DCHECK(b->fd == -1) << "we don't support splicing yet";
-    const auto* buf = b->buf.get();
-    do {
-      rv.append(reinterpret_cast<const char*>(buf->data()), buf->length());
-      buf = buf->next();
-    } while (buf != b->buf.get());
-  }
-  return rv;
-}
-
-} // namespace eden
-} // namespace facebook

eden/fs/fuse/BufVec.h

@@ -6,7 +6,6 @@
  */
 
 #pragma once
-#include <folly/FBVector.h>
 #include <folly/io/IOBuf.h>
 
 namespace facebook {
@@ -15,51 +14,14 @@ namespace eden {
 /**
  * Represents data that may come from a buffer or a file descriptor.
  *
- * While we don't currently have a fuse client lib that supports this,
- * we want to make sure we're ready to use it, so this looks like
- * a dumb wrapper around IOBuf at the moment.
+ * EdenFS does not currently support splicing between the FUSE device
+ * pipe and the backing files in the overlay, but there's an opportunity
+ * to improve performance on large files by enabling FUSE_CAP_SPLICE_READ or
+ * FUSE_CAP_SPLICE_WRITE.
+ *
+ * So pretend we have a type that corresponds roughly to libfuse's fuse_bufvec.
  */
-class BufVec {
-  struct Buf {
-    std::unique_ptr<folly::IOBuf> buf;
-    int fd{-1};
-    size_t fd_size{0};
-    off_t fd_pos{-1};
-
-    Buf(const Buf&) = delete;
-    Buf& operator=(const Buf&) = delete;
-    Buf(Buf&&) = default;
-    Buf& operator=(Buf&&) = default;
-
-    explicit Buf(std::unique_ptr<folly::IOBuf> buf);
-  };
-  folly::fbvector<std::shared_ptr<Buf>> items_;
-
- public:
-  BufVec(const BufVec&) = delete;
-  BufVec& operator=(const BufVec&) = delete;
-  BufVec(BufVec&&) = default;
-  BufVec& operator=(BufVec&&) = default;
-
-  explicit BufVec(std::unique_ptr<folly::IOBuf> buf);
-
-  /**
-   * Return an iovector suitable for e.g. writev()
-   *   auto iov = buf->getIov();
-   *   auto xfer = writev(fd, iov.data(), iov.size());
-   */
-  folly::fbvector<struct iovec> getIov() const;
-
-  /**
-   * Returns the total number of bytes in the BufVec.
-   */
-  size_t size() const;
-
-  /**
-   * Copies the buffer into a std::string.
-   */
-  std::string copyData() const;
-};
+using BufVec = std::unique_ptr<folly::IOBuf>;
 
 } // namespace eden
 } // namespace facebook

eden/fs/fuse/FuseChannel.cpp

@@ -397,6 +397,21 @@ void FuseChannel::sendReply(
   sendRawReply(vec.data(), vec.size());
 }
 
+void FuseChannel::sendReply(
+    const fuse_in_header& request,
+    const folly::IOBuf& buf) const {
+  fuse_out_header out;
+  out.unique = request.unique;
+  out.error = 0;
+
+  folly::fbvector<iovec> vec;
+  vec.reserve(1 + buf.countChainElements());
+  vec.push_back(make_iovec(out));
+  buf.appendToIov(&vec);
+
+  sendRawReply(vec.data(), vec.size());
+}
+
 void FuseChannel::sendReply(
     const fuse_in_header& request,
     folly::ByteRange bytes) const {
@@ -1373,8 +1388,7 @@ folly::Future<folly::Unit> FuseChannel::fuseRead(
 
   auto ino = InodeNumber{header->nodeid};
   return dispatcher_->read(ino, read->size, read->offset, RequestData::get())
-      .thenValue(
-          [](BufVec&& buf) { RequestData::get().sendReply(buf.getIov()); });
+      .thenValue([](BufVec&& buf) { RequestData::get().sendReply(*buf); });
 }
 
 folly::Future<folly::Unit> FuseChannel::fuseWrite(
@@ -1446,16 +1460,12 @@ folly::Future<folly::Unit> FuseChannel::fuseReadLink(
     const fuse_in_header* header,
     const uint8_t* /*arg*/) {
   XLOG(DBG7) << "FUSE_READLINK inode=" << header->nodeid;
-  return dispatcher_
-      ->readlink(
-          InodeNumber{header->nodeid},
-  /*kernelCachesReadlink=*/
+  bool kernelCachesReadlink = false;
 #ifdef FUSE_CACHE_SYMLINKS
-          connInfo_->flags & FUSE_CACHE_SYMLINKS
-#else
-          false
+  kernelCachesReadlink = connInfo_->flags & FUSE_CACHE_SYMLINKS;
 #endif
-          )
+  return dispatcher_
+      ->readlink(InodeNumber{header->nodeid}, kernelCachesReadlink)
       .thenValue([](std::string&& str) {
         RequestData::get().sendReply(folly::StringPiece(str));
       });

eden/fs/fuse/FuseChannel.h

@@ -258,6 +258,11 @@ class FuseChannel {
    */
   void sendReply(const fuse_in_header& request, folly::ByteRange bytes) const;
 
+  void sendReply(const fuse_in_header& request, folly::StringPiece bytes)
+      const {
+    sendReply(request, folly::ByteRange{bytes});
+  }
+
   /**
    * Sends a reply to a kernel request, consisting of multiple parts.
    * The `vec` parameter holds an array of payload components and is moved
@@ -270,6 +275,15 @@ class FuseChannel {
   void sendReply(const fuse_in_header& request, folly::fbvector<iovec>&& vec)
       const;
 
+  /**
+   * Sends a reply to a kernel request potentially consisting of multiple
+   * segments.
+   *
+   * throws system_error if the write fails.  Writes can fail if the
+   * data we send to the kernel is invalid.
+   */
+  void sendReply(const fuse_in_header& request, const folly::IOBuf& buf) const;
+
   /**
    * Sends a reply to the kernel.
    * The payload parameter is typically a fuse_out_XXX struct as defined

eden/fs/fuse/RequestData.h

@@ -190,21 +190,12 @@ class RequestData : public folly::RequestData, public ObjectFetchContext {
 
   template <typename T>
   void sendReply(const T& payload) {
-    static_assert(std::is_standard_layout_v<T>);
-    static_assert(std::is_trivial_v<T>);
     channel_->sendReply(stealReq(), payload);
   }
 
-  void sendReply(folly::ByteRange bytes) {
-    channel_->sendReply(stealReq(), bytes);
-  }
-
-  void sendReply(folly::fbvector<iovec>&& vec) {
-    channel_->sendReply(stealReq(), std::move(vec));
-  }
-
-  void sendReply(folly::StringPiece piece) {
-    channel_->sendReply(stealReq(), folly::ByteRange(piece));
+  template <typename T>
+  void sendReply(T&& payload) {
+    channel_->sendReply(stealReq(), std::forward<T>(payload));
   }
 
   // Reply with a negative errno value or 0 for success

eden/fs/fuse/test/BufVecTest.cpp

@@ -1,19 +0,0 @@
-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This software may be used and distributed according to the terms of the
- * GNU General Public License version 2.
- */
-
-#include "eden/fs/fuse/BufVec.h"
-
-#include <gtest/gtest.h>
-
-TEST(BufVecTest, BufVec) {
-  auto root = folly::IOBuf::wrapBuffer("hello", 5);
-  root->appendChain(folly::IOBuf::wrapBuffer("world", 5));
-  const auto bufVec = facebook::eden::BufVec{std::move(root)};
-  EXPECT_EQ(10u, bufVec.size());
-  EXPECT_EQ(10u, bufVec.copyData().size());
-  EXPECT_EQ("helloworld", bufVec.copyData());
-}

eden/fs/inodes/FileInode.cpp

@@ -864,7 +864,7 @@ folly::Future<size_t> FileInode::write(BufVec&& buf, off_t off) {
       nullptr,
       [buf = std::move(buf), off, self = inodePtrFromThis()](
           LockedState&& state) {
-        auto vec = buf.getIov();
+        auto vec = buf->getIov();
         return self->writeImpl(state, vec.data(), vec.size(), off);
       });
 }

eden/fs/inodes/FileInode.h

@@ -6,7 +6,6 @@
  */
 
 #pragma once
-#include <folly/File.h>
 #include <folly/Synchronized.h>
 #include <folly/futures/Future.h>
 #include <folly/futures/SharedPromise.h>
@@ -30,7 +29,7 @@ namespace facebook {
 namespace eden {
 
 class Blob;
-class BufVec;
+using BufVec = std::unique_ptr<folly::IOBuf>;
 class Hash;
 class ObjectFetchContext;
 class ObjectStore;

eden/fs/inodes/test/FileInodeTest.cpp

@@ -448,7 +448,7 @@ TEST(FileInode, readDuringLoad) {
 
   // The read() operation should have completed now.
   ASSERT_TRUE(dataFuture.isReady());
-  EXPECT_EQ(contents, std::move(dataFuture).get().copyData());
+  EXPECT_EQ(contents, std::move(dataFuture).get()->moveToFbString());
 }
 
 TEST(FileInode, writeDuringLoad) {
@@ -499,14 +499,15 @@ TEST(FileInode, truncateDuringLoad) {
 
   // The read should complete now too.
   ASSERT_TRUE(dataFuture.isReady());
-  EXPECT_EQ("", std::move(dataFuture).get().copyData());
+  EXPECT_EQ("", std::move(dataFuture).get()->moveToFbString());
 
   // For good measure, test reading and writing some more.
   inode->write("foobar\n"_sp, 5).get(0ms);
 
   dataFuture = inode->read(4096, 0, ObjectFetchContext::getNullContext());
   ASSERT_TRUE(dataFuture.isReady());
-  EXPECT_EQ("\0\0\0\0\0foobar\n"_sp, std::move(dataFuture).get().copyData());
+  EXPECT_EQ(
+      "\0\0\0\0\0foobar\n"_sp, std::move(dataFuture).get()->moveToFbString());
 
   EXPECT_FILE_INODE(inode, "\0\0\0\0\0foobar\n"_sp, 0644);
 }