From a1f3036fc8bcce5c17e790ecb44e6b8e580b77cf Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 7 Sep 2022 10:32:28 -0400 Subject: [PATCH] Reduce some allocations in glue --- crates/glue/src/types.rs | 110 +++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 52 deletions(-) diff --git a/crates/glue/src/types.rs b/crates/glue/src/types.rs index 79b39bc0d5..6c56d61d1a 100644 --- a/crates/glue/src/types.rs +++ b/crates/glue/src/types.rs @@ -1222,19 +1222,16 @@ fn add_tag_union<'a>( } } NonNullableUnwrapped(_) => { - let mut tags = - union_tags_to_types(&name, union_tags, subs, env, types, layout, true); - - debug_assert_eq!(tags.len(), 1); - - let (tag_name, opt_payload) = tags.pop().unwrap(); + let (tag_name, payload_vars) = single_tag_payload(union_tags, subs); + let (tag_name, opt_payload) = + tag_to_type(&name, env, tag_name, payload_vars, types, layout, true); // A recursive tag union with just one constructor // Optimization: No need to store a tag ID (the payload is "unwrapped") // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` RocTagUnion::NonNullableUnwrapped { name: name.clone(), - tag_name, + tag_name: tag_name.to_string(), payload: opt_payload.unwrap(), } } @@ -1318,7 +1315,7 @@ fn add_tag_union<'a>( // e.g. `RoseTree a : [Tree a (List (RoseTree a))]` RocTagUnion::SingleTagStruct { name: name.clone(), - tag_name, + tag_name: tag_name.to_string(), payload_fields, } } @@ -1334,7 +1331,7 @@ fn add_tag_union<'a>( RocTagUnion::SingleTagStruct { name: name.clone(), - tag_name, + tag_name: tag_name.to_string(), payload_fields: vec![type_id], } } @@ -1344,7 +1341,7 @@ fn add_tag_union<'a>( RocTagUnion::SingleTagStruct { name: name.clone(), - tag_name, + tag_name: tag_name.to_string(), payload_fields, } } @@ -1402,31 +1399,44 @@ fn union_tags_to_types<'a>( (name_str, payload_vars.to_vec()) }) .collect(); + // Sort tags alphabetically by tag name tags.sort_by(|(name1, _), (name2, _)| name1.cmp(name2)); - tags_to_types(name, tags, env, types, layout, is_recursive) + tags.into_iter() + .map(|(tag_name, payload_vars)| { + tag_to_type( + name, + env, + tag_name, + &payload_vars, + types, + layout, + is_recursive, + ) + }) + .collect() } fn single_tag_payload<'a>( union_tags: &'a UnionLabels, subs: &'a Subs, -) -> (String, &'a [Variable]) { +) -> (&'a str, &'a [Variable]) { let mut iter = union_tags.iter_from_subs(subs); let (tag_name, payload_vars) = iter.next().unwrap(); // This should be a single-tag union. debug_assert_eq!(iter.next(), None); - (tag_name.0.as_str().to_string(), payload_vars) + (tag_name.0.as_str(), payload_vars) } -fn single_tag_payload_fields<'a>( - union_tags: &UnionLabels, - subs: &Subs, +fn single_tag_payload_fields<'a, 'b>( + union_tags: &'b UnionLabels, + subs: &'b Subs, field_layouts: &[Layout<'a>], env: &mut Env<'a>, types: &mut Types, -) -> (String, Vec) { +) -> (&'b str, Vec) { let (tag_name, payload_vars) = single_tag_payload(union_tags, subs); let payload_fields: Vec = payload_vars @@ -1438,48 +1448,44 @@ fn single_tag_payload_fields<'a>( (tag_name, payload_fields) } -fn tags_to_types<'a>( +fn tag_to_type<'a, D: Display>( name: &str, - tags: Vec<(String, Vec)>, env: &mut Env<'a>, + tag_name: D, + payload_vars: &[Variable], types: &mut Types, layout: Layout<'a>, is_recursive: bool, -) -> Vec<(String, Option)> { - tags.into_iter() - .map(|(tag_name, payload_vars)| { - match struct_fields_needed(env, payload_vars.iter().copied()) { - 0 => { - // no payload - (tag_name, None) - } - 1 if !is_recursive => { - // this isn't recursive and there's 1 payload item, so it doesn't - // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them - // can have payloads of plain old Str, no struct wrapper needed. - let payload_var = payload_vars.get(0).unwrap(); - let payload_layout = env - .layout_cache - .from_var(env.arena, *payload_var, env.subs) - .expect("Something weird ended up in the content"); - let payload_id = add_type_help(env, payload_layout, *payload_var, None, types); +) -> (D, Option) { + match struct_fields_needed(env, payload_vars.iter().copied()) { + 0 => { + // no payload + (tag_name, None) + } + 1 if !is_recursive => { + // this isn't recursive and there's 1 payload item, so it doesn't + // need its own struct - e.g. for `[Foo Str, Bar Str]` both of them + // can have payloads of plain old Str, no struct wrapper needed. + let payload_var = payload_vars.get(0).unwrap(); + let payload_layout = env + .layout_cache + .from_var(env.arena, *payload_var, env.subs) + .expect("Something weird ended up in the content"); + let payload_id = add_type_help(env, payload_layout, *payload_var, None, types); - (tag_name, Some(payload_id)) - } - _ => { - // create a RocType for the payload and save it - let struct_name = format!("{}_{}", &name, tag_name); // e.g. "MyUnion_MyVariant" - let fields = payload_vars.iter().copied().enumerate(); - let struct_id = - add_struct(env, struct_name, fields, types, layout, |name, fields| { - RocType::TagUnionPayload { name, fields } - }); + (tag_name, Some(payload_id)) + } + _ => { + // create a RocType for the payload and save it + let struct_name = format!("{}_{}", &name, tag_name); // e.g. "MyUnion_MyVariant" + let fields = payload_vars.iter().copied().enumerate(); + let struct_id = add_struct(env, struct_name, fields, types, layout, |name, fields| { + RocType::TagUnionPayload { name, fields } + }); - (tag_name, Some(struct_id)) - } - } - }) - .collect() + (tag_name, Some(struct_id)) + } + } } fn struct_fields_needed>(env: &mut Env<'_>, vars: I) -> usize {