Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes ConcurrentSlab to no longer clone values #5313

Merged
merged 3 commits into from
Dec 2, 2023
Merged

Changes ConcurrentSlab to no longer clone values #5313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0db4a83
Changes `ConcurrentSlab` to no longer clone values
esdrubal Nov 29, 2023
b5ab97e
Removes clone from TypeEngine
esdrubal Nov 30, 2023
55f439d
Changed deref() to *.
esdrubal Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions forc-plugins/forc-doc/src/doc/descriptor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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());

Expand Down
2 changes: 1 addition & 1 deletion forc-plugins/forc-doc/src/doc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions forc-plugins/forc-doc/src/render/item/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
);
Expand Down
4 changes: 2 additions & 2 deletions forc-plugins/forc-doc/src/render/item/type_anchor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)?;
Expand All @@ -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,
)?)
Expand Down
5 changes: 4 additions & 1 deletion sway-core/src/abi_generation/evm_abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}",
Expand Down
24 changes: 14 additions & 10 deletions sway-core/src/abi_generation/fuel_abi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
..
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
78 changes: 24 additions & 54 deletions sway-core/src/concurrent_slab.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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,
}
Expand All @@ -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);
}
}
5 changes: 3 additions & 2 deletions sway-core/src/control_flow_analysis/analyze_return_paths.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
Loading