-
Notifications
You must be signed in to change notification settings - Fork 26.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rewrite IdFactory and IdFactoryWithReuse (vercel/turborepo#8769)
### Description The real goal here was to extend `IdFactory<T>` to work with 64-bit ids, which I'll need soon for globally unique (and non-reusable) "execution ids" (vercel/turborepo#8771) to support the safety requirements of local uncached Vcs. I got a little carried away and essentially rewrote this: - (Debatable if this is an improvement or not) ID generation re-use requires an almost-but-not-entirely-free check of the concurrent queue, so it is now opt-in using the `IdFactoryWithReuse`. - ID generation is always performed with an AtomicU64 (which shouldn't really be any more or less expensive than AtomicU32 on 64 bit architectures). - u32 overflow detection is performed by using a `TryFrom` conversion from a NonZeroU64. Previously we could only detect and panic on the first id generated after overflow. Now we should detect and panic on (basically) all ids generated after overflow. - New versions of `concurrent-queue` make the `unbounded` constructor `const`, which allows us to eliminate the use of `Lazy`. - Add a unit test for overflow detection ### Testing Instructions ``` cargo nextest r -p turbo-tasks -p turbo-tasks-memory ```
- Loading branch information
Showing
7 changed files
with
121 additions
and
29 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,44 +1,118 @@ | ||
| use std::{ | ||
| any::type_name, | ||
| marker::PhantomData, | ||
| ops::Deref, | ||
| sync::atomic::{AtomicU32, Ordering}, | ||
| num::NonZeroU64, | ||
| sync::atomic::{AtomicU64, Ordering}, | ||
| }; | ||
|
|
||
| use concurrent_queue::ConcurrentQueue; | ||
| use once_cell::sync::Lazy; | ||
|
|
||
| /// A helper for constructing id types like [`FunctionId`][crate::FunctionId]. | ||
| /// | ||
| /// For ids that may be re-used, see [`IdFactoryWithReuse`]. | ||
| pub struct IdFactory<T> { | ||
| next_id: AtomicU32, | ||
| free_ids: Lazy<ConcurrentQueue<T>>, | ||
| phantom_data: PhantomData<T>, | ||
| next_id: AtomicU64, | ||
| _phantom_data: PhantomData<T>, | ||
| } | ||
|
|
||
| impl<T: From<u32> + Deref<Target = u32>> Default for IdFactory<T> { | ||
| impl<T> IdFactory<T> { | ||
| pub const fn new() -> Self { | ||
| Self { | ||
| next_id: AtomicU64::new(1), | ||
| _phantom_data: PhantomData, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<T> Default for IdFactory<T> { | ||
| fn default() -> Self { | ||
| Self::new() | ||
| } | ||
| } | ||
|
|
||
| impl<T: From<u32> + Deref<Target = u32>> IdFactory<T> { | ||
| impl<T> IdFactory<T> | ||
| where | ||
| T: TryFrom<NonZeroU64>, | ||
| { | ||
| /// Return a unique new id. | ||
| /// | ||
| /// Panics (best-effort) if the id type overflows. | ||
| pub fn get(&self) -> T { | ||
| // Safety: u64 will not overflow. This is *very* unlikely to happen (would take | ||
| // decades). | ||
| let new_id = | ||
| unsafe { NonZeroU64::new_unchecked(self.next_id.fetch_add(1, Ordering::Relaxed)) }; | ||
|
|
||
| // Use the extra bits of the AtomicU64 as cheap overflow detection when the | ||
| // value is less than 64 bits. | ||
| match new_id.try_into() { | ||
| Ok(id) => id, | ||
| Err(_) => panic!( | ||
| "Overflow detected while attempting to generate a unique {}", | ||
| type_name::<T>(), | ||
| ), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// An [`IdFactory`], but extended with a free list to allow for id reuse for | ||
| /// ids such as [`BackendJobId`][crate::backend::BackendJobId]. | ||
| pub struct IdFactoryWithReuse<T> { | ||
| factory: IdFactory<T>, | ||
| free_ids: ConcurrentQueue<T>, | ||
| } | ||
|
|
||
| impl<T> IdFactoryWithReuse<T> { | ||
| pub const fn new() -> Self { | ||
| Self { | ||
| next_id: AtomicU32::new(1), | ||
| free_ids: Lazy::new(|| ConcurrentQueue::unbounded()), | ||
| phantom_data: PhantomData, | ||
| factory: IdFactory::new(), | ||
| free_ids: ConcurrentQueue::unbounded(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<T> Default for IdFactoryWithReuse<T> { | ||
| fn default() -> Self { | ||
| Self::new() | ||
| } | ||
| } | ||
|
|
||
| impl<T> IdFactoryWithReuse<T> | ||
| where | ||
| T: TryFrom<NonZeroU64>, | ||
| { | ||
| /// Return a new or potentially reused id. | ||
| /// | ||
| /// Panics (best-effort) if the id type overflows. | ||
| pub fn get(&self) -> T { | ||
| if let Ok(id) = self.free_ids.pop() { | ||
| return id; | ||
| } | ||
| self.next_id.fetch_add(1, Ordering::Relaxed).into() | ||
| self.free_ids.pop().unwrap_or_else(|_| self.factory.get()) | ||
| } | ||
|
|
||
| /// Add an id to the free list, allowing it to be re-used on a subsequent | ||
| /// call to [`IdFactoryWithReuse::get`]. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// It must be ensured that the id is no longer used | ||
| pub unsafe fn reuse(&self, id: T) { | ||
| let _ = self.free_ids.push(id); | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use std::num::NonZeroU8; | ||
|
|
||
| use super::*; | ||
|
|
||
| #[test] | ||
| #[should_panic(expected = "Overflow detected")] | ||
| fn test_overflow() { | ||
| let factory = IdFactory::<NonZeroU8>::new(); | ||
| assert_eq!(factory.get(), NonZeroU8::new(1).unwrap()); | ||
| assert_eq!(factory.get(), NonZeroU8::new(2).unwrap()); | ||
| for _i in 2..256 { | ||
| factory.get(); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters