remove some unsafe code (#366)

This commit modifies the cache code to use Box::leak, eliminating the need for std::mem::transmute.
This commit is contained in:
Daniel Gulotta 2025-07-29 13:23:08 -07:00 committed by GitHub
parent d6c4d9e943
commit ce8cabc337
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 7 additions and 12 deletions

View file

@ -7,7 +7,7 @@ use plonky2::{
witness::{PartitionWitness, Witness}, witness::{PartitionWitness, Witness},
}, },
plonk::circuit_data::CommonCircuitData, plonk::circuit_data::CommonCircuitData,
util::serialization::{Buffer, IoResult, Read, Write}, util::serialization::{Buffer, IoError, IoResult, Read, Write},
}; };
/// Plonky2 generator that allows debugging values assigned to targets. This generator doesn't /// Plonky2 generator that allows debugging values assigned to targets. This generator doesn't
@ -66,7 +66,7 @@ impl<F: RichField + Extendable<D>, const D: usize> SimpleGenerator<F, D> for Deb
let name_len = src.read_usize()?; let name_len = src.read_usize()?;
let mut name_buf = vec![0; name_len]; let mut name_buf = vec![0; name_len];
src.read_exact(&mut name_buf)?; src.read_exact(&mut name_buf)?;
let name = unsafe { String::from_utf8_unchecked(name_buf) }; let name = String::from_utf8(name_buf).map_err(|_| IoError)?;
let xs = src.read_target_vec()?; let xs = src.read_target_vec()?;
Ok(Self { name, xs }) Ok(Self { name, xs })
} }

15
src/cache/mem.rs vendored
View file

@ -10,7 +10,7 @@ use serde::{de::DeserializeOwned, Serialize};
use sha2::{Digest, Sha256}; use sha2::{Digest, Sha256};
#[allow(clippy::type_complexity)] #[allow(clippy::type_complexity)]
static CACHE: LazyLock<Mutex<HashMap<String, Option<Box<dyn Any + Send + 'static>>>>> = static CACHE: LazyLock<Mutex<HashMap<String, Option<&'static (dyn Any + Sync)>>>> =
LazyLock::new(|| Mutex::new(HashMap::new())); LazyLock::new(|| Mutex::new(HashMap::new()));
pub struct CacheEntry<T: 'static> { pub struct CacheEntry<T: 'static> {
@ -28,7 +28,7 @@ impl<T> Deref for CacheEntry<T> {
/// Get the artifact named `name` from the memory cache. If it doesn't exist, it will be built by /// Get the artifact named `name` from the memory cache. If it doesn't exist, it will be built by
/// calling `build_fn` and stored. /// calling `build_fn` and stored.
/// The artifact is indexed by `params: P`. /// The artifact is indexed by `params: P`.
pub(crate) fn get<T: Serialize + DeserializeOwned + Send + 'static, P: Serialize>( pub(crate) fn get<T: Serialize + DeserializeOwned + Sync + 'static, P: Serialize>(
name: &str, name: &str,
params: &P, params: &P,
build_fn: fn(&P) -> T, build_fn: fn(&P) -> T,
@ -43,14 +43,9 @@ pub(crate) fn get<T: Serialize + DeserializeOwned + Send + 'static, P: Serialize
let mut cache = CACHE.lock()?; let mut cache = CACHE.lock()?;
if let Some(entry) = cache.get(&key) { if let Some(entry) = cache.get(&key) {
if let Some(boxed_data) = entry { if let Some(boxed_data) = entry {
if let Some(data) = boxed_data.downcast_ref::<T>() { if let Some(data) = (*boxed_data as &dyn Any).downcast_ref::<T>() {
log::debug!("found {} in the mem cache", name); log::debug!("found {} in the mem cache", name);
// The data is now in the heap (boxed), and will never go away because we can return Ok(CacheEntry { value: data });
// only insert into the CACHE if there's no entry, we can't delete nor update.
// Since it's not going away, not moving, and the CACHE is 'static, it's safe
// to extend the lifetime of data to 'static.
let data_static = unsafe { std::mem::transmute::<&T, &'static T>(data) };
return Ok(CacheEntry { value: data_static });
} else { } else {
panic!( panic!(
"type={} doesn't match the type in the cached boxed value with name={}", "type={} doesn't match the type in the cached boxed value with name={}",
@ -76,7 +71,7 @@ pub(crate) fn get<T: Serialize + DeserializeOwned + Send + 'static, P: Serialize
let elapsed = std::time::Instant::now() - start; let elapsed = std::time::Instant::now() - start;
log::debug!("built {} in {:?}", name, elapsed); log::debug!("built {} in {:?}", name, elapsed);
CACHE.lock()?.insert(key, Some(Box::new(data))); CACHE.lock()?.insert(key, Some(Box::leak(Box::new(data))));
// Call `get` again and this time we'll retrieve the data from the cache // Call `get` again and this time we'll retrieve the data from the cache
return get(name, params, build_fn); return get(name, params, build_fn);
} }