Skip to content

Commit 4f12ac3

Browse files
authored
Fixes compiler performance in mega_example. (#5841)
## Description ReplaceDecls for method application was calling replace_decls twice for FunctionDecl. This was visiting function declarations as a binary tree, doing replace decls twice per FunctionDecl. With these changes, we merge both decl mappings before calling replace_decls once instead of twice. The performance dropped from over 2min(did not wait for test to finish) to 7 seconds in the mega example. With --experimental-new-encoding and --release. ## Checklist - [x] I have linked to any relevant issues. - [x] 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). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] 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.
1 parent 35c810e commit 4f12ac3

File tree

7 files changed

+148
-41
lines changed

7 files changed

+148
-41
lines changed

sway-core/src/decl_engine/mapping.rs

+4
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ impl DeclMapping {
8484
self.mapping.is_empty()
8585
}
8686

87+
pub(crate) fn extend(&mut self, other: &DeclMapping) {
88+
self.mapping.extend(other.mapping.clone());
89+
}
90+
8791
pub(crate) fn from_interface_and_item_and_impld_decl_refs(
8892
interface_decl_refs: InterfaceItemMap,
8993
item_decl_refs: ItemMap,

sway-core/src/decl_engine/ref.rs

-30
Original file line numberDiff line numberDiff line change
@@ -163,36 +163,6 @@ where
163163
}
164164
}
165165

166-
impl<T> DeclRef<DeclId<T>>
167-
where
168-
AssociatedItemDeclId: From<DeclId<T>>,
169-
DeclEngine: DeclEngineIndex<T>,
170-
T: Named + Spanned + ReplaceDecls + std::fmt::Debug + Clone,
171-
{
172-
/// Returns Ok(None), if nothing was replaced.
173-
/// Ok(true), if something was replaced.
174-
/// and errors when appropriated.
175-
pub(crate) fn replace_decls_and_insert_new_with_parent(
176-
&self,
177-
decl_mapping: &DeclMapping,
178-
handler: &Handler,
179-
ctx: &mut TypeCheckContext,
180-
) -> Result<Option<Self>, ErrorEmitted> {
181-
let decl_engine = ctx.engines().de();
182-
183-
let original = decl_engine.get(&self.id);
184-
185-
let mut new = (*original).clone();
186-
let changed = new.replace_decls(decl_mapping, handler, ctx)?;
187-
188-
Ok(changed.then(|| {
189-
decl_engine
190-
.insert(new)
191-
.with_parent(decl_engine, self.id.into())
192-
}))
193-
}
194-
}
195-
196166
impl<T> EqWithEngines for DeclRef<DeclId<T>>
197167
where
198168
DeclEngine: DeclEngineIndex<T>,

sway-core/src/language/ty/expression/expression_variant.rs

+3-11
Original file line numberDiff line numberDiff line change
@@ -784,18 +784,8 @@ impl ReplaceDecls for TyExpressionVariant {
784784
} => {
785785
let mut has_changes = false;
786786

787-
// TODO do we need this call?
788-
// The next call seems to make this one redundant
789787
has_changes |= fn_ref.replace_decls(decl_mapping, handler, ctx)?;
790788

791-
if let Some(new_decl_ref) = fn_ref
792-
.clone()
793-
.replace_decls_and_insert_new_with_parent(decl_mapping, handler, ctx)?
794-
{
795-
fn_ref.replace_id(*new_decl_ref.id());
796-
has_changes = true;
797-
};
798-
799789
for (_, arg) in arguments.iter_mut() {
800790
if let Ok(r) = arg.replace_decls(decl_mapping, handler, ctx) {
801791
has_changes |= r;
@@ -830,7 +820,7 @@ impl ReplaceDecls for TyExpressionVariant {
830820

831821
// Handle the trait constraints. This includes checking to see if the trait
832822
// constraints are satisfied and replacing old decl ids based on the
833-
let inner_decl_mapping =
823+
let mut inner_decl_mapping =
834824
TypeParameter::gather_decl_mapping_from_trait_constraints(
835825
handler,
836826
ctx.by_ref(),
@@ -839,6 +829,8 @@ impl ReplaceDecls for TyExpressionVariant {
839829
&method.name.span(),
840830
)?;
841831

832+
inner_decl_mapping.extend(decl_mapping);
833+
842834
if method.replace_decls(&inner_decl_mapping, handler, ctx)? {
843835
decl_engine.replace(*fn_ref.id(), method);
844836
has_changes = true;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
[[package]]
2+
name = "core"
3+
source = "path+from-root-A63EC0723332CE0D"
4+
5+
[[package]]
6+
name = "mega_example"
7+
source = "member"
8+
dependencies = ["std"]
9+
10+
[[package]]
11+
name = "std"
12+
source = "path+from-root-A63EC0723332CE0D"
13+
dependencies = ["core"]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[project]
2+
authors = ["Fuel Labs <contact@fuel.sh>"]
3+
license = "Apache-2.0"
4+
name = "mega_example"
5+
entry = "main.sw"
6+
7+
[dependencies]
8+
std = { path = "../../../../../../../sway-lib-std" }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
contract;
2+
3+
use std::hash::*;
4+
5+
struct SimpleGeneric<T> {
6+
single_generic_param: T,
7+
}
8+
9+
struct PassTheGenericOn<K> {
10+
one: SimpleGeneric<K>,
11+
}
12+
13+
struct StructWArrayGeneric<L> {
14+
a: [L; 2],
15+
}
16+
17+
struct StructWTupleGeneric<M> {
18+
a: (M, M),
19+
}
20+
21+
struct StructWDiffTupleGeneric<M, N> {
22+
a: (M, N),
23+
}
24+
25+
enum EnumWGeneric<N> {
26+
a: u64,
27+
b: N,
28+
}
29+
30+
struct MegaExample<T, U> {
31+
a: ([U; 2], T),
32+
b: Vec<([EnumWGeneric<StructWTupleGeneric<StructWArrayGeneric<PassTheGenericOn<T>>>>; 1], u32)>,
33+
}
34+
35+
abi MyContract {
36+
fn struct_w_generic(arg1: SimpleGeneric<u64>) -> SimpleGeneric<u64>;
37+
fn struct_delegating_generic(arg1: PassTheGenericOn<str[3]>) -> PassTheGenericOn<str[3]>;
38+
fn struct_w_generic_in_array(arg1: StructWArrayGeneric<u32>) -> StructWArrayGeneric<u32>;
39+
fn struct_w_generic_in_tuple(arg1: StructWTupleGeneric<u32>) -> StructWTupleGeneric<u32>;
40+
fn struct_w_diff_generic_in_tuple(arg1: StructWDiffTupleGeneric<u32, bool>) -> StructWDiffTupleGeneric<u32, bool>;
41+
42+
fn enum_w_generic(arg1: EnumWGeneric<u64>) -> EnumWGeneric<u64>;
43+
44+
fn complex_test(arg1: MegaExample<str[2], b256>);
45+
}
46+
47+
impl Hash for str[3] {
48+
fn hash(self, ref mut state: Hasher) {
49+
state.write_str_array(self);
50+
}
51+
}
52+
53+
impl MyContract for Contract {
54+
fn struct_w_generic(arg1: SimpleGeneric<u64>) -> SimpleGeneric<u64> {
55+
let expected = SimpleGeneric {
56+
single_generic_param: 123u64,
57+
};
58+
59+
assert(arg1.single_generic_param == expected.single_generic_param);
60+
61+
expected
62+
}
63+
64+
fn struct_delegating_generic(arg1: PassTheGenericOn<str[3]>) -> PassTheGenericOn<str[3]> {
65+
let expected = PassTheGenericOn {
66+
one: SimpleGeneric {
67+
single_generic_param: __to_str_array("abc"),
68+
},
69+
};
70+
71+
assert(sha256(expected.one.single_generic_param) == sha256(arg1.one.single_generic_param));
72+
73+
expected
74+
}
75+
76+
fn struct_w_generic_in_array(arg1: StructWArrayGeneric<u32>) -> StructWArrayGeneric<u32> {
77+
let expected = StructWArrayGeneric {
78+
a: [1u32, 2u32],
79+
};
80+
81+
assert(expected.a[0] == arg1.a[0]);
82+
assert(expected.a[1] == arg1.a[1]);
83+
84+
expected
85+
}
86+
87+
fn struct_w_generic_in_tuple(arg1: StructWTupleGeneric<u32>) -> StructWTupleGeneric<u32> {
88+
let expected = StructWTupleGeneric { a: (1u32, 2u32) };
89+
assert(expected.a.0 == arg1.a.0);
90+
assert(expected.a.1 == arg1.a.1);
91+
92+
expected
93+
}
94+
95+
fn struct_w_diff_generic_in_tuple(
96+
arg1: StructWDiffTupleGeneric<u32, bool>,
97+
) -> StructWDiffTupleGeneric<u32, bool> {
98+
let expected = StructWDiffTupleGeneric { a: (1u32, false) };
99+
assert(expected.a.0 == arg1.a.0);
100+
assert(expected.a.1 == arg1.a.1);
101+
102+
expected
103+
}
104+
fn enum_w_generic(arg1: EnumWGeneric<u64>) -> EnumWGeneric<u64> {
105+
match arg1 {
106+
EnumWGeneric::b(value) => {
107+
assert(value == 10u64);
108+
}
109+
_ => {
110+
assert(false)
111+
}
112+
}
113+
EnumWGeneric::b(10)
114+
}
115+
116+
fn complex_test(arg1: MegaExample<str[2], b256>) {}
117+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
category = "compile"
2+
validate_abi = false
3+
expected_warnings = 4

0 commit comments

Comments
 (0)