backingstore: use nullptr for aux-not-found

Summary:
Use nullptr intead of exceptions to represent the low level "not found" state for file and tree aux data. liubov-dmitrieva discovered through profiling that we spend a lot of CPU time on exceptions. In particular, we first try a local-only fetch, and if the aux data doesn't exist, we raise an exception and then enqueue the aux data to be fetched in allow-remote mode.

I changed the low level SaplingNativeBackingStore::get{Blob,Tree}AuxData to use nullptr to mean not-found, and then updated callers.

Note that I did not change the batch aux codepaths. Since these typically fetch remotely, "not found" results are not expected (so it is okay to keep using exceptions).

Reviewed By: liubov-dmitrieva

Differential Revision: D67545966

fbshipit-source-id: 73e8fa7a2584192faa13295870bff6c38eec0727

Author: muirdm

eden/fs/store/hg/SaplingBackingStore.cpp

@@ -1169,7 +1169,7 @@ SaplingBackingStore::getTreeAuxData(
       ObjectFetchContext::ObjectType::TreeAuxData);
 
   auto auxData = getLocalTreeAuxData(proxyHash);
-  if (auxData.hasValue()) {
+  if (auxData.hasValue() && auxData.value()) {
     stats_->increment(&SaplingBackingStoreStats::fetchTreeAuxDataSuccess);
     stats_->increment(&SaplingBackingStoreStats::fetchTreeAuxDataLocal);
     return folly::makeSemiFuture(GetTreeAuxResult{
@@ -1241,10 +1241,14 @@ folly::Try<TreeAuxDataPtr> SaplingBackingStore::getLocalTreeAuxData(
   using GetTreeAuxDataResult = folly::Try<TreeAuxDataPtr>;
 
   if (auxData.hasValue()) {
-    return GetTreeAuxDataResult{
-        std::make_shared<TreeAuxDataPtr::element_type>(TreeAuxData{
-            Hash32{std::move(auxData.value()->digest_hash)},
-            auxData.value()->digest_size})};
+    if (auxData.value()) {
+      return GetTreeAuxDataResult{
+          std::make_shared<TreeAuxDataPtr::element_type>(TreeAuxData{
+              Hash32{std::move(auxData.value()->digest_hash)},
+              auxData.value()->digest_size})};
+    } else {
+      return GetTreeAuxDataResult{nullptr};
+    }
   } else {
     return GetTreeAuxDataResult{auxData.exception()};
   }
@@ -1495,7 +1499,7 @@ SaplingBackingStore::getBlobAuxData(
       ObjectFetchContext::ObjectType::BlobAuxData);
 
   auto auxData = getLocalBlobAuxData(proxyHash);
-  if (auxData.hasValue()) {
+  if (auxData.hasValue() && auxData.value()) {
     stats_->increment(&SaplingBackingStoreStats::fetchBlobAuxDataSuccess);
     stats_->increment(&SaplingBackingStoreStats::fetchBlobAuxDataLocal);
     return folly::makeSemiFuture(GetBlobAuxResult{
@@ -1572,11 +1576,15 @@ folly::Try<BlobAuxDataPtr> SaplingBackingStore::getLocalBlobAuxData(
   using GetBlobAuxDataResult = folly::Try<BlobAuxDataPtr>;
 
   if (auxData.hasValue()) {
-    return GetBlobAuxDataResult{
-        std::make_shared<BlobAuxDataPtr::element_type>(BlobAuxData{
-            Hash20{std::move(auxData.value()->content_sha1)},
-            Hash32{std::move(auxData.value()->content_blake3)},
-            auxData.value()->total_size})};
+    if (auxData.value()) {
+      return GetBlobAuxDataResult{
+          std::make_shared<BlobAuxDataPtr::element_type>(BlobAuxData{
+              Hash20{std::move(auxData.value()->content_sha1)},
+              Hash32{std::move(auxData.value()->content_blake3)},
+              auxData.value()->total_size})};
+    } else {
+      return GetBlobAuxDataResult{nullptr};
+    }
   } else {
     return GetBlobAuxDataResult{auxData.exception()};
   }

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

@@ -118,14 +118,10 @@ SaplingNativeBackingStore::getTreeAuxData(NodeId node, bool local) {
       "Importing tree aux data node={} from hgcache",
       folly::hexlify(node));
   return folly::makeTryWith([&] {
-    auto auxData = sapling_backingstore_get_tree_aux(
+    return sapling_backingstore_get_tree_aux(
         *store_.get(),
         rust::Slice<const uint8_t>{node.data(), node.size()},
         fetch_mode);
-    XCHECK(
-        auxData.get(),
-        "sapling_backingstore_get_tree_aux returned a nullptr, but did not throw an exception.");
-    return auxData;
   });
 }
 
@@ -220,14 +216,10 @@ SaplingNativeBackingStore::getBlobAuxData(NodeId node, bool local) {
       "Importing blob aux data node={} from hgcache",
       folly::hexlify(node));
   return folly::makeTryWith([&] {
-    auto auxData = sapling_backingstore_get_file_aux(
+    return sapling_backingstore_get_file_aux(
         *store_.get(),
         rust::Slice<const uint8_t>{node.data(), node.size()},
         fetch_mode);
-    XCHECK(
-        auxData.get(),
-        "sapling_backingstore_get_file_aux returned a nullptr, but did not throw an exception.");
-    return auxData;
   });
 }
 

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

@@ -303,12 +303,10 @@ pub fn sapling_backingstore_get_tree_aux(
     node: &[u8],
     fetch_mode: ffi::FetchMode,
 ) -> Result<SharedPtr<ffi::TreeAuxData>> {
-    Ok(SharedPtr::new(
-        store
-            .get_tree_aux(node, FetchMode::from(fetch_mode))
-            .and_then(|opt| opt.ok_or_else(|| Error::msg("no tree aux data found")))?
-            .into(),
-    ))
+    match store.get_tree_aux(node, FetchMode::from(fetch_mode))? {
+        Some(aux) => Ok(SharedPtr::new(aux.into())),
+        None => Ok(SharedPtr::null()),
+    }
 }
 
 pub fn sapling_backingstore_get_tree_aux_batch(
@@ -373,12 +371,10 @@ pub fn sapling_backingstore_get_file_aux(
     node: &[u8],
     fetch_mode: ffi::FetchMode,
 ) -> Result<SharedPtr<ffi::FileAuxData>> {
-    Ok(SharedPtr::new(
-        store
-            .get_file_aux(node, FetchMode::from(fetch_mode))
-            .and_then(|opt| opt.ok_or_else(|| Error::msg("no file aux data found")))?
-            .into(),
-    ))
+    match store.get_file_aux(node, FetchMode::from(fetch_mode))? {
+        Some(aux) => Ok(SharedPtr::new(aux.into())),
+        None => Ok(SharedPtr::null()),
+    }
 }
 
 pub fn sapling_backingstore_get_file_aux_batch(