FuelLabs / sway

🌴 Empowering everyone to build reliable and efficient smart contracts.
https://docs.fuel.network/docs/sway/
Apache License 2.0
62.71k stars 5.36k forks source link

Remove interior mutability in the ConcurrentSlab #6180

Open JoshuaBatty opened 3 months ago

JoshuaBatty commented 3 months ago

Currently the ConcurrentSlab has an RwLock. As we don't do any concurrent processing in the compiler we should remove interior mutability. When profiling, the compiler seems to be spending ~80% of it's time on atomic operations around this type. Removing this should lead to a significant performance boost.

pub(crate) struct ConcurrentSlab<T> {
    pub inner: RwLock<Inner<T>>,
}

pub fn insert(&self, value: T) -> usize {
    self.insert_arc(Arc::new(value))
}

This should be changed to remove the RwLock and methods that mutate state should take &mut self.

pub(crate) struct ConcurrentSlab<T> {
    pub inner: Inner<T>,
}

// change these methods to take &mut self
pub fn insert(&mut self, value: T) -> usize {
    self.insert_arc(Arc::new(value))
}
tritao commented 3 months ago

Additionally we should really remove the Arc altogether which defeats the purpose of why this was introduced in the first place (arena pattern), and leads to really poor cache locality. I'm planning to look into that once I have some time, so no worries if it's not on your radar for now.

JoshuaBatty commented 3 months ago

Awesome, I don't think we will have the scope to work on this anymore from toolings side. It seems like we should try and not store a reference to engines in the Type check context so we can pass around &mut Engines when things needs mutating.