Back to all reviewers

avoid unnecessary Arc wrapping

cloudflare/workerd
Based on 2 comments
Rust

Don't wrap atomic types in Arc unless you need to share ownership across multiple owners. Atomic types like AtomicBool, AtomicU64, etc. are already thread-safe and can be used directly in structs that will be wrapped in Arc at a higher level.

Concurrency Rust

Reviewer Prompt

Don’t wrap atomic types in Arc unless you need to share ownership across multiple owners. Atomic types like AtomicBool, AtomicU64, etc. are already thread-safe and can be used directly in structs that will be wrapped in Arc at a higher level.

Using Arc creates unnecessary indirection and allocation overhead when AtomicBool alone provides the required thread safety. The Arc should be applied at the struct level that contains the atomic field, not around individual atomic fields.

Example of the anti-pattern:

struct Impl {
    sender: mpsc::SyncSender<Vec<u8>>,
    write_shutdown: Arc<std::sync::atomic::AtomicBool>, // Unnecessary Arc
}

Preferred approach:

struct Impl {
    sender: mpsc::SyncSender<Vec<u8>>,
    write_shutdown: std::sync::atomic::AtomicBool, // Direct atomic type
}

// Wrap the entire struct in Arc when sharing is needed
let impl_instance = Arc::new(Impl::new(...));

This reduces memory overhead, eliminates unnecessary heap allocation, and simplifies the code while maintaining the same thread safety guarantees.

2
Comments Analyzed
Rust
Primary Language
Concurrency
Category

Source Discussions