Changes ConcurrentSlab to no longer clone values (#5313)

## Description `ConcurrentSlab` now stores and returns an Arc, and on `get` it only clones an Arc which is much cheaper than cloning the whole values. The type engine is still cloning every TypeInfo we should be able to also avoid that and get further improvements in speed. With these changes core + std-lib compilation improved from around 3 secs to 1.5 secs. `cargo run --bin test --release` is running in around 8min versus 16min in master. ## Checklist - [ ] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
FuelLabs · Dec 2, 2023 · 1e12d5d · 1e12d5d
1 parent e70cb77
commit 1e12d5d
Show file tree

Hide file tree

Showing 69 changed files with 667 additions and 644 deletions.
diff --git a/forc-plugins/forc-doc/src/doc/descriptor.rs b/forc-plugins/forc-doc/src/doc/descriptor.rs
@@ -18,7 +18,7 @@ trait RequiredMethods {
 impl RequiredMethods for Vec<DeclRefTraitFn> {
     fn to_methods(&self, decl_engine: &DeclEngine) -> Vec<TyTraitFn> {
         self.iter()
-            .map(|decl_ref| decl_engine.get_trait_fn(decl_ref))
+            .map(|decl_ref| (*decl_engine.get_trait_fn(decl_ref)).clone())
             .collect()
     }
 }
@@ -45,12 +45,12 @@ impl Descriptor {
                 if !document_private_items && struct_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
-                    let item_name = struct_decl.call_path.suffix;
+                    let item_name = struct_decl.call_path.suffix.clone();
                     let attrs_opt = (!struct_decl.attributes.is_empty())
                         .then(|| struct_decl.attributes.to_html_string());
                     let context = (!struct_decl.fields.is_empty()).then_some(Context::new(
                         module_info.clone(),
-                        ContextType::StructFields(struct_decl.fields),
+                        ContextType::StructFields(struct_decl.fields.clone()),
                     ));
 
                     Ok(Descriptor::Documentable(Document {
@@ -82,12 +82,12 @@ impl Descriptor {
                 if !document_private_items && enum_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
-                    let item_name = enum_decl.call_path.suffix;
+                    let item_name = enum_decl.call_path.suffix.clone();
                     let attrs_opt = (!enum_decl.attributes.is_empty())
                         .then(|| enum_decl.attributes.to_html_string());
                     let context = (!enum_decl.variants.is_empty()).then_some(Context::new(
                         module_info.clone(),
-                        ContextType::EnumVariants(enum_decl.variants),
+                        ContextType::EnumVariants(enum_decl.variants.clone()),
                     ));
 
                     Ok(Descriptor::Documentable(Document {
@@ -115,7 +115,7 @@ impl Descriptor {
                 }
             }
             ty::TyDecl::TraitDecl(ty::TraitDecl { decl_id, .. }) => {
-                let trait_decl = decl_engine.get_trait(decl_id);
+                let trait_decl = (*decl_engine.get_trait(decl_id)).clone();
                 if !document_private_items && trait_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
@@ -163,7 +163,7 @@ impl Descriptor {
                 }
             }
             ty::TyDecl::AbiDecl(ty::AbiDecl { decl_id, .. }) => {
-                let abi_decl = decl_engine.get_abi(decl_id);
+                let abi_decl = (*decl_engine.get_abi(decl_id)).clone();
                 let item_name = abi_decl.name;
                 let attrs_opt =
                     (!abi_decl.attributes.is_empty()).then(|| abi_decl.attributes.to_html_string());
@@ -212,7 +212,7 @@ impl Descriptor {
                     .then(|| storage_decl.attributes.to_html_string());
                 let context = (!storage_decl.fields.is_empty()).then_some(Context::new(
                     module_info.clone(),
-                    ContextType::StorageFields(storage_decl.fields),
+                    ContextType::StorageFields(storage_decl.fields.clone()),
                 ));
 
                 Ok(Descriptor::Documentable(Document {
@@ -243,7 +243,7 @@ impl Descriptor {
                 if !document_private_items && fn_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
-                    let item_name = fn_decl.name;
+                    let item_name = fn_decl.name.clone();
                     let attrs_opt = (!fn_decl.attributes.is_empty())
                         .then(|| fn_decl.attributes.to_html_string());
 
@@ -276,7 +276,7 @@ impl Descriptor {
                 if !document_private_items && const_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
-                    let item_name = const_decl.call_path.suffix;
+                    let item_name = const_decl.call_path.suffix.clone();
                     let attrs_opt = (!const_decl.attributes.is_empty())
                         .then(|| const_decl.attributes.to_html_string());
 

diff --git a/forc-plugins/forc-doc/src/doc/mod.rs b/forc-plugins/forc-doc/src/doc/mod.rs
@@ -122,7 +122,7 @@ impl Documentation {
             if let TyAstNodeContent::Declaration(ref decl) = ast_node.content {
                 if let TyDecl::ImplTrait(impl_trait) = decl {
                     impl_traits.push((
-                        decl_engine.get_impl_trait(&impl_trait.decl_id),
+                        (*decl_engine.get_impl_trait(&impl_trait.decl_id)).clone(),
                         module_info.clone(),
                     ))
                 } else {

diff --git a/forc-plugins/forc-doc/src/render/item/context.rs b/forc-plugins/forc-doc/src/render/item/context.rs
@@ -53,7 +53,7 @@ impl Renderable for Context {
                 for field in fields {
                     let struct_field_id = format!("structfield.{}", field.name.as_str());
                     let type_anchor = render_type_anchor(
-                        render_plan.engines.te().get(field.type_argument.type_id),
+                        (*render_plan.engines.te().get(field.type_argument.type_id)).clone(),
                         &render_plan,
                         &self.module_info,
                     );
@@ -81,7 +81,7 @@ impl Renderable for Context {
                 for field in fields {
                     let storage_field_id = format!("storagefield.{}", field.name.as_str());
                     let type_anchor = render_type_anchor(
-                        render_plan.engines.te().get(field.type_argument.type_id),
+                        (*render_plan.engines.te().get(field.type_argument.type_id)).clone(),
                         &render_plan,
                         &self.module_info,
                     );
@@ -109,7 +109,7 @@ impl Renderable for Context {
                 for variant in variants {
                     let enum_variant_id = format!("variant.{}", variant.name.as_str());
                     let type_anchor = render_type_anchor(
-                        render_plan.engines.te().get(variant.type_argument.type_id),
+                        (*render_plan.engines.te().get(variant.type_argument.type_id)).clone(),
                         &render_plan,
                         &self.module_info,
                     );

diff --git a/forc-plugins/forc-doc/src/render/item/type_anchor.rs b/forc-plugins/forc-doc/src/render/item/type_anchor.rs
@@ -23,7 +23,7 @@ pub(crate) fn render_type_anchor(
     match type_info {
         TypeInfo::Array(ty_arg, len) => {
             let inner = render_type_anchor(
-                render_plan.engines.te().get(ty_arg.type_id),
+                (*render_plan.engines.te().get(ty_arg.type_id)).clone(),
                 render_plan,
                 current_module_info,
             )?;
@@ -37,7 +37,7 @@ pub(crate) fn render_type_anchor(
             let mut rendered_args: Vec<_> = Vec::new();
             for ty_arg in ty_args {
                 rendered_args.push(render_type_anchor(
-                    render_plan.engines.te().get(ty_arg.type_id),
+                    (*render_plan.engines.te().get(ty_arg.type_id)).clone(),
                     render_plan,
                     current_module_info,
                 )?)

diff --git a/sway-core/src/abi_generation/evm_abi.rs b/sway-core/src/abi_generation/evm_abi.rs
@@ -40,7 +40,10 @@ fn get_type_str(
             abi_str(&type_engine.get(*type_id), type_engine, decl_engine)
         )
     } else {
-        match (type_engine.get(*type_id), type_engine.get(resolved_type_id)) {
+        match (
+            &*type_engine.get(*type_id),
+            &*type_engine.get(resolved_type_id),
+        ) {
             (TypeInfo::Custom { .. }, TypeInfo::Struct { .. }) => {
                 format!(
                     "struct {}",

diff --git a/sway-core/src/abi_generation/fuel_abi.rs b/sway-core/src/abi_generation/fuel_abi.rs
@@ -266,7 +266,10 @@ impl TypeId {
                     .abi_str(ctx, type_engine, decl_engine)
             )
         } else {
-            match (type_engine.get(*self), type_engine.get(resolved_type_id)) {
+            match (
+                &*type_engine.get(*self),
+                &*type_engine.get(resolved_type_id),
+            ) {
                 (TypeInfo::Custom { .. }, TypeInfo::Struct { .. }) => type_engine
                     .get(resolved_type_id)
                     .abi_str(ctx, type_engine, decl_engine),
@@ -339,9 +342,9 @@ impl TypeId {
         types: &mut Vec<program_abi::TypeDeclaration>,
         resolved_type_id: TypeId,
     ) -> Option<Vec<program_abi::TypeApplication>> {
-        match type_engine.get(*self) {
+        match &*type_engine.get(*self) {
             TypeInfo::Enum(decl_ref) => {
-                let decl = decl_engine.get_enum(&decl_ref);
+                let decl = decl_engine.get_enum(decl_ref);
                 // A list of all `program_abi::TypeDeclaration`s needed for the enum variants
                 let variants = decl
                     .variants
@@ -392,7 +395,7 @@ impl TypeId {
                 )
             }
             TypeInfo::Struct(decl_ref) => {
-                let decl = decl_engine.get_struct(&decl_ref);
+                let decl = decl_engine.get_struct(decl_ref);
 
                 // A list of all `program_abi::TypeDeclaration`s needed for the struct fields
                 let field_types = decl
@@ -444,7 +447,7 @@ impl TypeId {
                 )
             }
             TypeInfo::Array(..) => {
-                if let TypeInfo::Array(elem_ty, _) = type_engine.get(resolved_type_id) {
+                if let TypeInfo::Array(elem_ty, _) = &*type_engine.get(resolved_type_id) {
                     // The `program_abi::TypeDeclaration`s needed for the array element type
                     let elem_abi_ty = program_abi::TypeDeclaration {
                         type_id: elem_ty.initial_type_id.index(),
@@ -489,7 +492,7 @@ impl TypeId {
                 }
             }
             TypeInfo::Tuple(_) => {
-                if let TypeInfo::Tuple(fields) = type_engine.get(resolved_type_id) {
+                if let TypeInfo::Tuple(fields) = &*type_engine.get(resolved_type_id) {
                     // A list of all `program_abi::TypeDeclaration`s needed for the tuple fields
                     let fields_types = fields
                         .iter()
@@ -545,6 +548,7 @@ impl TypeId {
                 if !self.is_generic_parameter(type_engine, decl_engine, resolved_type_id) {
                     // A list of all `program_abi::TypeDeclaration`s needed for the type arguments
                     let type_args = type_arguments
+                        .clone()
                         .unwrap_or_default()
                         .iter()
                         .zip(
@@ -591,7 +595,7 @@ impl TypeId {
                 }
             }
             TypeInfo::Alias { .. } => {
-                if let TypeInfo::Alias { ty, .. } = type_engine.get(resolved_type_id) {
+                if let TypeInfo::Alias { ty, .. } = &*type_engine.get(resolved_type_id) {
                     ty.initial_type_id.get_abi_type_components(
                         ctx,
                         type_engine,
@@ -621,7 +625,7 @@ impl TypeId {
         resolved_type_id: TypeId,
     ) -> Option<Vec<program_abi::TypeApplication>> {
         let resolved_params = resolved_type_id.get_type_parameters(type_engine, decl_engine);
-        match type_engine.get(*self) {
+        match &*type_engine.get(*self) {
             TypeInfo::Custom {
                 type_arguments: Some(type_arguments),
                 ..
@@ -672,7 +676,7 @@ impl TypeId {
                     .collect::<Vec<_>>()
             }),
             TypeInfo::Enum(decl_ref) => {
-                let decl = decl_engine.get_enum(&decl_ref);
+                let decl = decl_engine.get_enum(decl_ref);
                 // Here, type_id for each type parameter should contain resolved types
                 let abi_type_arguments = decl
                     .type_parameters
@@ -722,7 +726,7 @@ impl TypeId {
             }
 
             TypeInfo::Struct(decl_ref) => {
-                let decl = decl_engine.get_struct(&decl_ref);
+                let decl = decl_engine.get_struct(decl_ref);
                 // Here, type_id for each type parameter should contain resolved types
                 let abi_type_arguments = decl
                     .type_parameters

diff --git a/sway-core/src/concurrent_slab.rs b/sway-core/src/concurrent_slab.rs
@@ -1,10 +1,11 @@
-use crate::{decl_engine::*, engine_threading::*, type_system::*};
-use std::{fmt, sync::RwLock};
-use sway_types::{Named, Spanned};
+use std::{
+    fmt,
+    sync::{Arc, RwLock},
+};
 
 #[derive(Debug)]
 pub(crate) struct ConcurrentSlab<T> {
-    inner: RwLock<Vec<T>>,
+    inner: RwLock<Vec<Arc<T>>>,
 }
 
 impl<T> Clone for ConcurrentSlab<T>
@@ -27,12 +28,6 @@ impl<T> Default for ConcurrentSlab<T> {
     }
 }
 
-impl<T> ConcurrentSlab<T> {
-    pub fn with_slice<R>(&self, run: impl FnOnce(&[T]) -> R) -> R {
-        run(&self.inner.read().unwrap())
-    }
-}
-
 pub struct ListDisplay<I> {
     pub list: I,
 }
@@ -57,63 +52,38 @@ impl<T> ConcurrentSlab<T>
 where
     T: Clone,
 {
+    pub fn len(&self) -> usize {
+        let inner = self.inner.read().unwrap();
+        inner.len()
+    }
+
     pub fn insert(&self, value: T) -> usize {
         let mut inner = self.inner.write().unwrap();
         let ret = inner.len();
-        inner.push(value);
+        inner.push(Arc::new(value));
         ret
     }
 
-    pub fn get(&self, index: usize) -> T {
-        let inner = self.inner.read().unwrap();
-        inner[index].clone()
-    }
-
-    pub fn retain(&self, predicate: impl Fn(&T) -> bool) {
+    pub fn insert_arc(&self, value: Arc<T>) -> usize {
         let mut inner = self.inner.write().unwrap();
-        inner.retain(predicate);
+        let ret = inner.len();
+        inner.push(value);
+        ret
     }
-}
-
-impl ConcurrentSlab<TypeSourceInfo> {
-    pub fn replace(
-        &self,
-        index: TypeId,
-        prev_value: &TypeSourceInfo,
-        new_value: TypeSourceInfo,
-        engines: &Engines,
-    ) -> Option<TypeSourceInfo> {
-        let index = index.index();
-        // The comparison below ends up calling functions in the slab, which
-        // can lead to deadlocks if we used a single read/write lock.
-        // So we split the operation: we do the read only operations with
-        // a single scoped read lock below, and only after the scope do
-        // we get a write lock for writing into the slab.
-        {
-            let inner = self.inner.read().unwrap();
-            let actual_prev_value = &inner[index];
-            if !actual_prev_value
-                .type_info
-                .eq(&prev_value.type_info, engines)
-            {
-                return Some(actual_prev_value.clone());
-            }
-        }
 
+    pub fn replace(&self, index: usize, new_value: T) -> Option<T> {
         let mut inner = self.inner.write().unwrap();
-        inner[index] = new_value;
+        inner[index] = Arc::new(new_value);
         None
     }
-}
 
-impl<T> ConcurrentSlab<T>
-where
-    DeclEngine: DeclEngineIndex<T>,
-    T: Named + Spanned,
-{
-    pub fn replace(&self, index: DeclId<T>, new_value: T) -> Option<T> {
+    pub fn get(&self, index: usize) -> Arc<T> {
+        let inner = self.inner.read().unwrap();
+        inner[index].clone()
+    }
+
+    pub fn retain(&self, predicate: impl Fn(&Arc<T>) -> bool) {
         let mut inner = self.inner.write().unwrap();
-        inner[index.inner()] = new_value;
-        None
+        inner.retain(predicate);
     }
 }
diff --git a/sway-core/src/control_flow_analysis/analyze_return_paths.rs b/sway-core/src/control_flow_analysis/analyze_return_paths.rs
@@ -220,15 +220,16 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
             Ok(leaves.to_vec())
         }
         ty::TyDecl::ImplTrait(ty::ImplTrait { decl_id, .. }) => {
+            let impl_trait = decl_engine.get_impl_trait(decl_id);
             let ty::TyImplTrait {
                 trait_name, items, ..
-            } = decl_engine.get_impl_trait(decl_id);
+            } = &*impl_trait;
             let entry_node = graph.add_node(ControlFlowGraphNode::from_node(node));
             for leaf in leaves {
                 graph.add_edge(*leaf, entry_node, "".into());
             }
 
-            connect_impl_trait(engines, &trait_name, graph, &items, entry_node)?;
+            connect_impl_trait(engines, trait_name, graph, items, entry_node)?;
             Ok(leaves.to_vec())
         }
         ty::TyDecl::ErrorRecovery(..) => Ok(leaves.to_vec()),