backingstore: directly return IOBuf in FFI layer without intermediately converting to Vec<u8>

Summary: Change `backingstore` FFI layer to directly return `IOBuf` instead of intermediately converting to `Vec<u8>` and back to `IOBuf` again in the C++ code.

Reviewed By: liubov-dmitrieva

Differential Revision: D72132358

fbshipit-source-id: ff5d94f39d17ac381a44aa4424289add5040203a

Author: YousefSalama

eden/scm/lib/backingstore/BUCK

@@ -92,6 +92,8 @@ rust_library(
         "fbsource//third-party/rust:parking_lot",
         "fbsource//third-party/rust:tracing",
         "fbsource//third-party/rust:tracing-subscriber",
+        "//common/rust/folly:iobuf",
+        "//eden/scm/lib/blob:scm-blob",
         "//eden/scm/lib/config/loader:configloader",
         "//eden/scm/lib/constructors:constructors",
         "//eden/scm/lib/eagerepo:eagerepo",

eden/scm/lib/backingstore/Cargo.toml

@@ -33,6 +33,7 @@ sapling-repo = { version = "0.1.0", path = "../repo" }
 sapling-storemodel = { version = "0.1.0", path = "../storemodel" }
 sapling-tracing-collector = { version = "0.1.0", path = "../tracing-collector" }
 sapling-types = { version = "0.1.0", path = "../types" }
+scm-blob = { version = "0.1.0", path = "../blob" }
 tracing = { version = "0.1.41", features = ["attributes", "valuable"] }
 tracing-subscriber = { version = "0.3.18", features = ["chrono", "env-filter", "json", "local-time", "parking_lot", "registry"] }
 

eden/scm/lib/backingstore/include/ffi.h

@@ -89,7 +89,7 @@ void sapling_backingstore_get_blob_batch_handler(
     std::shared_ptr<GetBlobBatchResolver> resolver,
     size_t index,
     rust::String error,
-    rust::Box<Blob> blob);
+    std::unique_ptr<folly::IOBuf> blob);
 
 void sapling_backingstore_get_file_aux_batch_handler(
     std::shared_ptr<GetFileAuxBatchResolver> resolver,

eden/scm/lib/backingstore/src/SaplingNativeBackingStore.cpp

@@ -158,23 +158,10 @@ folly::Try<std::unique_ptr<folly::IOBuf>> SaplingNativeBackingStore::getBlob(
     FetchMode fetch_mode) {
   XLOGF(DBG7, "Importing blob node={} from hgcache", folly::hexlify(node));
   return folly::makeTryWith([&] {
-    auto opt_blob = sapling_backingstore_get_blob(
+    return sapling_backingstore_get_blob(
         *store_.get(),
         rust::Slice<const uint8_t>{node.data(), node.size()},
         fetch_mode);
-
-    if (!opt_blob.present) {
-      return std::unique_ptr<folly::IOBuf>(nullptr);
-    } else {
-      auto blob = opt_blob.blob.into_raw();
-      return folly::IOBuf::takeOwnership(
-          reinterpret_cast<void*>(blob->bytes.data()),
-          blob->bytes.size(),
-          [](void* /* buf */, void* blob) mutable {
-            auto vec = rust::Box<Blob>::from_raw(reinterpret_cast<Blob*>(blob));
-          },
-          reinterpret_cast<void*>(blob));
-    }
   });
 }
 

eden/scm/lib/backingstore/src/backingstore.rs

@@ -31,8 +31,8 @@ use log::warn;
 use metrics::ods;
 use repo::repo::Repo;
 use repo::RepoMinimalInfo;
+use scm_blob::ScmBlob;
 use storemodel::BoxIterator;
-use storemodel::Bytes;
 use storemodel::FileAuxData;
 use storemodel::FileStore;
 use storemodel::TreeAuxData;
@@ -176,7 +176,7 @@ impl BackingStore {
     }
 
     #[instrument(level = "trace", skip(self))]
-    pub fn get_blob(&self, fctx: FetchContext, node: &[u8]) -> Result<Option<Vec<u8>>> {
+    pub fn get_blob(&self, fctx: FetchContext, node: &[u8]) -> Result<Option<ScmBlob>> {
         self.maybe_reload().filestore.single(fctx, node)
     }
 
@@ -186,7 +186,7 @@ impl BackingStore {
     #[instrument(level = "trace", skip(self, resolve))]
     pub fn get_blob_batch<F>(&self, fctx: FetchContext, keys: Vec<Key>, resolve: F)
     where
-        F: Fn(usize, Result<Option<Vec<u8>>>),
+        F: Fn(usize, Result<Option<ScmBlob>>),
     {
         self.maybe_reload()
             .filestore
@@ -567,24 +567,19 @@ where
 }
 
 /// Read file content.
-impl LocalRemoteImpl<Bytes, Vec<u8>> for Arc<dyn FileStore> {
-    fn get_local_single(&self, path: &RepoPath, id: HgId) -> Result<Option<Bytes>> {
-        Ok(self
-            .get_local_content(path, id)?
-            .map(|blob| blob.into_bytes()))
+impl LocalRemoteImpl<ScmBlob> for Arc<dyn FileStore> {
+    fn get_local_single(&self, path: &RepoPath, id: HgId) -> Result<Option<ScmBlob>> {
+        self.get_local_content(path, id)
     }
-    fn get_single(&self, fctx: FetchContext, path: &RepoPath, id: HgId) -> Result<Bytes> {
-        Ok(self.get_content(fctx, path, id)?.into_bytes())
+    fn get_single(&self, fctx: FetchContext, path: &RepoPath, id: HgId) -> Result<ScmBlob> {
+        self.get_content(fctx, path, id)
     }
     fn get_batch_iter(
         &self,
         fctx: FetchContext,
         keys: Vec<Key>,
-    ) -> Result<BoxIterator<Result<(Key, Bytes)>>> {
-        Ok(Box::new(
-            self.get_content_iter(fctx, keys)?
-                .map(|r| r.map(|(k, v)| (k, v.into_bytes()))),
-        ))
+    ) -> Result<BoxIterator<Result<(Key, ScmBlob)>>> {
+        Ok(Box::new(self.get_content_iter(fctx, keys)?))
     }
 }
 

eden/scm/lib/backingstore/src/ffi.cpp

@@ -52,21 +52,13 @@ void sapling_backingstore_get_blob_batch_handler(
     std::shared_ptr<GetBlobBatchResolver> resolver,
     size_t index,
     rust::String error,
-    rust::Box<Blob> blob) {
+    std::unique_ptr<folly::IOBuf> blob) {
   using ResolveResult = folly::Try<std::unique_ptr<folly::IOBuf>>;
 
   resolver->resolve(
       index, folly::makeTryWith([&] {
         if (error.empty()) {
-          auto result = blob.into_raw();
-          return ResolveResult{folly::IOBuf::takeOwnership(
-              reinterpret_cast<void*>(result->bytes.data()),
-              result->bytes.size(),
-              [](void* /* buf */, void* blob) mutable {
-                auto box =
-                    rust::Box<Blob>::from_raw(reinterpret_cast<Blob*>(blob));
-              },
-              reinterpret_cast<void*>(result))};
+          return ResolveResult{std::move(blob)};
         } else {
           return ResolveResult{SaplingFetchError{std::string(error)}};
         }

eden/scm/lib/backingstore/src/ffi.rs

@@ -14,6 +14,8 @@ use anyhow::anyhow;
 use anyhow::Error;
 use anyhow::Result;
 use cxx::SharedPtr;
+use cxx::UniquePtr;
+use iobuf::IOBuf;
 use storemodel::FileAuxData as ScmStoreFileAuxData;
 use types::fetch_cause::FetchCause;
 use types::fetch_mode::FetchMode;
@@ -105,15 +107,6 @@ pub(crate) mod ffi {
         files: Vec<String>,
     }
 
-    pub struct Blob {
-        pub(crate) bytes: Vec<u8>,
-    }
-
-    pub struct OptionalBlob {
-        present: bool,
-        blob: Box<Blob>,
-    }
-
     pub struct FileAuxData {
         total_size: u64,
         content_sha1: [u8; 20],
@@ -123,6 +116,9 @@ pub(crate) mod ffi {
     unsafe extern "C++" {
         include!("eden/scm/lib/backingstore/include/ffi.h");
 
+        #[namespace = "folly"]
+        type IOBuf = iobuf::IOBuf;
+
         type GetTreeBatchResolver;
         type GetTreeAuxBatchResolver;
         type GetBlobBatchResolver;
@@ -146,7 +142,7 @@ pub(crate) mod ffi {
             resolve_state: SharedPtr<GetBlobBatchResolver>,
             index: usize,
             error: String,
-            blob: Box<Blob>,
+            blob: UniquePtr<IOBuf>,
         );
 
         unsafe fn sapling_backingstore_get_file_aux_batch_handler(
@@ -202,7 +198,7 @@ pub(crate) mod ffi {
             store: &BackingStore,
             node: &[u8],
             fetch_mode: FetchMode,
-        ) -> Result<OptionalBlob>;
+        ) -> Result<UniquePtr<IOBuf>>;
 
         pub fn sapling_backingstore_get_blob_batch(
             store: &BackingStore,
@@ -403,20 +399,14 @@ pub fn sapling_backingstore_get_blob(
     store: &BackingStore,
     node: &[u8],
     fetch_mode: ffi::FetchMode,
-) -> Result<ffi::OptionalBlob> {
+) -> Result<UniquePtr<IOBuf>> {
     // the cause is not propagated for this API
     match store.get_blob(
         FetchContext::new_with_cause(FetchMode::from(fetch_mode), FetchCause::EdenUnknown),
         node,
     )? {
-        Some(blob) => Ok(ffi::OptionalBlob {
-            blob: Box::new(ffi::Blob { bytes: blob }),
-            present: true,
-        }),
-        None => Ok(ffi::OptionalBlob {
-            blob: Box::new(ffi::Blob { bytes: Vec::new() }),
-            present: false,
-        }),
+        Some(blob) => Ok(blob.into_iobuf().into()),
+        None => Ok(UniquePtr::null()),
     }
 }
 
@@ -436,11 +426,8 @@ pub fn sapling_backingstore_get_blob_batch(
             let result = result.and_then(|opt| opt.ok_or_else(|| Error::msg("no blob found")));
             let resolver = resolver.clone();
             let (error, blob) = match result {
-                Ok(blob) => (String::default(), Box::new(ffi::Blob { bytes: blob })),
-                Err(error) => (
-                    format!("{:?}", error),
-                    Box::new(ffi::Blob { bytes: vec![] }),
-                ),
+                Ok(blob) => (String::default(), blob.into_iobuf().into()),
+                Err(error) => (format!("{:?}", error), UniquePtr::null()),
             };
             unsafe { ffi::sapling_backingstore_get_blob_batch_handler(resolver, idx, error, blob) };
         },

eden/scm/lib/blob/src/lib.rs

@@ -33,6 +33,14 @@ impl ScmBlob {
         }
     }
 
+    #[cfg(fbcode_build)]
+    pub fn into_iobuf(self) -> iobuf::IOBufShared {
+        match self {
+            Self::Bytes(bytes) => iobuf::IOBufShared::from(bytes),
+            Self::IOBuf(buf) => buf,
+        }
+    }
+
     pub fn len(&self) -> usize {
         match self {
             Self::Bytes(bytes) => bytes.len(),