Skip to content

Commit

Permalink
fix has_with_results todo
Browse files Browse the repository at this point in the history
has_with_results, even less allocation
  • Loading branch information
KGrewal1 committed Nov 28, 2024
1 parent 36d3e35 commit e5d69d8
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 16 deletions.
15 changes: 6 additions & 9 deletions nativelink-store/src/filesystem_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -769,14 +769,11 @@ impl<Fe: FileEntry> StoreDriver for FilesystemStore<Fe> {
keys: &[StoreKey<'_>],
results: &mut [Option<u64>],
) -> Result<(), Error> {
// TODO(allada) This is a bit of a hack to get around the lifetime issues with the
// existence_cache. We need to convert the digests to owned values to be able to
// insert them into the cache. In theory it should be able to elide this conversion
// but it seems to be a bit tricky to get right.
let keys: Vec<StoreKey<'static>> = keys.iter().map(|v| v.borrow().into_owned()).collect();
self.evicting_map
.sizes_for_keys::<&Vec<StoreKey<'_>>, StoreKey<'static>, &StoreKey<'_>>(
&keys, results, false, /* peek */
.sizes_for_keys::<_, StoreKeyBorrow<'_>, &StoreKey<'_>>(
keys.iter(),
results,
false, /* peek */
)
.await;
// We need to do a special pass to ensure our zero files exist.
Expand Down Expand Up @@ -884,8 +881,8 @@ impl<Fe: FileEntry> StoreDriver for FilesystemStore<Fe> {
return Ok(());
}

let key_borrow: StoreKeyBorrow<'_> = key.borrow().into();
let entry = self.evicting_map.get(&key_borrow).await.ok_or_else(|| {
let key_borrow: &StoreKeyBorrow<'_> = std::borrow::Borrow::borrow(&key);
let entry = self.evicting_map.get(key_borrow).await.ok_or_else(|| {
make_err!(
Code::NotFound,
"{} not found in filesystem store here",
Expand Down
12 changes: 5 additions & 7 deletions nativelink-store/src/memory_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use nativelink_metric::MetricsComponent;
use nativelink_util::buf_channel::{DropCloserReadHalf, DropCloserWriteHalf};
use nativelink_util::evicting_map::{EvictingMap, LenEntry};
use nativelink_util::health_utils::{default_health_status_indicator, HealthStatusIndicator};
use nativelink_util::store_trait::{StoreDriver, StoreKey, UploadSizeInfo};
use nativelink_util::store_trait::{StoreDriver, StoreKey, StoreKeyBorrow, UploadSizeInfo};

use crate::cas_utils::is_zero_digest;

Expand Down Expand Up @@ -84,13 +84,11 @@ impl StoreDriver for MemoryStore {
keys: &[StoreKey<'_>],
results: &mut [Option<u64>],
) -> Result<(), Error> {
// TODO(allada): This is a dirty hack to get around the lifetime issues with the
// evicting map.
let digests: Vec<StoreKey<'static>> =
keys.iter().map(|key| key.borrow().into_owned()).collect();
self.evicting_map
.sizes_for_keys::<Vec<StoreKey<'_>>, StoreKey<'static>, StoreKey<'_>>(
digests, results, false, /* peek */
.sizes_for_keys::<_, StoreKeyBorrow<'_>, &StoreKey<'_>>(
keys.iter(),
results,
false, /* peek */
)
.await;
// We need to do a special pass to ensure our zero digest exist.
Expand Down
13 changes: 13 additions & 0 deletions nativelink-util/src/store_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,19 @@ where
}
}

impl<'a, 'b> Borrow<StoreKeyBorrow<'a>> for &StoreKey<'b>
where
'b: 'a,
{
fn borrow(&self) -> &StoreKeyBorrow<'a> {
// similar to the auto impl impl<T> Borrow<T> for &T
// but instead impl impl<U> Borrow<T> for &T
// for our new type key wrapper U
let y = *self;
std::borrow::Borrow::borrow(y)
}
}

/// Holds something that can be converted into a key the
/// store API can understand. Generally this is a digest
/// but it can also be a string if the caller wishes to
Expand Down

0 comments on commit e5d69d8

Please sign in to comment.