DigestInfo now does string conversions on the stack

We no longer make a few heap allocations to convert DigestInfo messages into strings. Now we can get a &str of a DigestInfo and do all the operations on the stack. In addition serde's [De]Serialization now treats DigestInfo as a string.
TraceMachina · Sep 8, 2024 · c47b53f · c47b53f
1 parent 140b7cb
commit c47b53f
Show file tree

Hide file tree

Showing 10 changed files with 366 additions and 54 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/nativelink-store/src/filesystem_store.rs b/nativelink-store/src/filesystem_store.rs
@@ -307,7 +307,7 @@ fn make_temp_digest(digest: &mut DigestInfo) {
             .fetch_add(1, Ordering::Relaxed)
             .to_le_bytes(),
     );
-    digest.set_packed_hash(hash);
+    digest.set_packed_hash(*hash);
 }
 
 impl LenEntry for FileEntryImpl {
@@ -595,13 +595,10 @@ impl<Fe: FileEntry> FilesystemStore<Fe> {
     }
 
     pub async fn get_file_entry_for_digest(&self, digest: &DigestInfo) -> Result<Arc<Fe>, Error> {
-        self.evicting_map.get(digest).await.ok_or_else(|| {
-            make_err!(
-                Code::NotFound,
-                "{} not found in filesystem store",
-                digest.hash_str()
-            )
-        })
+        self.evicting_map
+            .get(digest)
+            .await
+            .ok_or_else(|| make_err!(Code::NotFound, "{digest} not found in filesystem store"))
     }
 
     async fn update_file<'a>(
@@ -852,13 +849,10 @@ impl<Fe: FileEntry> StoreDriver for FilesystemStore<Fe> {
             return Ok(());
         }
 
-        let entry = self.evicting_map.get(&digest).await.ok_or_else(|| {
-            make_err!(
-                Code::NotFound,
-                "{} not found in filesystem store",
-                digest.hash_str()
-            )
-        })?;
+        let entry =
+            self.evicting_map.get(&digest).await.ok_or_else(|| {
+                make_err!(Code::NotFound, "{digest} not found in filesystem store")
+            })?;
         let read_limit = length.unwrap_or(usize::MAX) as u64;
         let mut resumeable_temp_file = entry.read_file_part(offset as u64, read_limit).await?;
 

diff --git a/nativelink-store/src/grpc_store.rs b/nativelink-store/src/grpc_store.rs
@@ -588,7 +588,7 @@ impl StoreDriver for GrpcStore {
             "{}/uploads/{}/blobs/{}/{}",
             &self.instance_name,
             Uuid::new_v4().hyphenated().encode_lower(&mut buf),
-            digest.hash_str(),
+            digest.packed_hash(),
             digest.size_bytes(),
         );
 
@@ -673,7 +673,7 @@ impl StoreDriver for GrpcStore {
         let resource_name = format!(
             "{}/blobs/{}/{}",
             &self.instance_name,
-            digest.hash_str(),
+            digest.packed_hash(),
             digest.size_bytes(),
         );
 

diff --git a/nativelink-store/src/verify_store.rs b/nativelink-store/src/verify_store.rs
@@ -21,6 +21,7 @@ use nativelink_metric::MetricsComponent;
 use nativelink_util::buf_channel::{
     make_buf_channel_pair, DropCloserReadHalf, DropCloserWriteHalf,
 };
+use nativelink_util::common::PackedHash;
 use nativelink_util::digest_hasher::{
     default_digest_hasher_func, DigestHasher, ACTIVE_HASHER_FUNC,
 };
@@ -61,7 +62,7 @@ impl VerifyStore {
         mut tx: DropCloserWriteHalf,
         mut rx: DropCloserReadHalf,
         maybe_expected_digest_size: Option<u64>,
-        original_hash: &[u8; 32],
+        original_hash: &PackedHash,
         mut maybe_hasher: Option<&mut D>,
     ) -> Result<(), Error> {
         let mut sum_size: u64 = 0;
@@ -119,9 +120,7 @@ impl VerifyStore {
                     if original_hash != hash_result {
                         self.hash_verification_failures.inc();
                         return Err(make_input_err!(
-                            "Hashes do not match, got: {} but digest hash was {}",
-                            hex::encode(original_hash),
-                            hex::encode(hash_result),
+                            "Hashes do not match, got: {original_hash} but digest hash was {hash_result}",
                         ));
                     }
                 }

diff --git a/nativelink-util/BUILD.bazel b/nativelink-util/BUILD.bazel
@@ -79,6 +79,7 @@ rust_test_suite(
     srcs = [
         "tests/buf_channel_test.rs",
         "tests/channel_body_for_tests_test.rs",
+        "tests/common_test.rs",
         "tests/evicting_map_test.rs",
         "tests/fastcdc_test.rs",
         "tests/fs_test.rs",
@@ -109,6 +110,7 @@ rust_test_suite(
         "@crates//:parking_lot",
         "@crates//:pretty_assertions",
         "@crates//:rand",
+        "@crates//:serde_json",
         "@crates//:sha2",
         "@crates//:tokio",
         "@crates//:tokio-stream",

diff --git a/nativelink-util/Cargo.toml b/nativelink-util/Cargo.toml
@@ -23,7 +23,7 @@ pin-project = "1.1.5"
 #                    Release PR: https://github.com/tokio-rs/console/pull/576
 console-subscriber = { git = "https://github.com/tokio-rs/console", rev = "5f6faa2" , default-features = false }
 futures = { version = "0.3.30", default-features = false }
-hex = { version = "0.4.3", default-features = false }
+hex = { version = "0.4.3", default-features = false, features = ["std"] }
 hyper = "1.4.1"
 hyper-util = "0.1.6"
 lru = { version = "0.12.3", default-features = false }
@@ -49,3 +49,4 @@ nativelink-macro = { path = "../nativelink-macro" }
 http-body-util = "0.1.2"
 pretty_assertions = { version = "1.4.0", features = ["std"] }
 rand = { version = "0.8.5", default-features = false }
+serde_json = { version = "1.0.128", default-features = false }