From 81acf08d67045252d3ed47d78de9d7f417daa927 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Wed, 16 Oct 2024 11:06:35 +0200 Subject: [PATCH] Use to_smolstr in a few more places to reduce allocation pressure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: ``` Benchmark 1: ./target/release/slint-viewer ../slint-perf/app.slint Time (mean ± σ): 626.1 ms ± 16.0 ms [User: 548.6 ms, System: 72.6 ms] Range (min … max): 610.9 ms … 653.0 ms 10 runs allocations: 3395727 temporary allocations: 466634 ``` After: ``` Benchmark 1: ./target/release/slint-viewer ../slint-perf/app.slint Time (mean ± σ): 622.6 ms ± 13.9 ms [User: 556.6 ms, System: 63.5 ms] Range (min … max): 609.7 ms … 645.4 ms 10 runs allocations: 3371931 temporary allocations: 459315 ``` --- internal/compiler/object_tree.rs | 8 ++++---- internal/compiler/passes/binding_analysis.rs | 4 +++- internal/compiler/passes/lower_property_to_element.rs | 4 ++-- internal/compiler/passes/unique_id.rs | 2 +- internal/compiler/typeloader.rs | 6 +++--- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/internal/compiler/object_tree.rs b/internal/compiler/object_tree.rs index a38644c6d8e..0108938bfe6 100644 --- a/internal/compiler/object_tree.rs +++ b/internal/compiler/object_tree.rs @@ -896,7 +896,7 @@ impl Element { ) -> ElementRc { let base_type = if let Some(base_node) = node.QualifiedName() { let base = QualifiedTypeName::from_node(base_node.clone()); - let base_string = base.to_string(); + let base_string = base.to_smolstr(); match parent_type.lookup_type_for_child_element(&base_string, tr) { Ok(ElementType::Component(c)) if c.is_global() => { diag.push_error( @@ -1509,7 +1509,7 @@ impl Element { } } - if r.borrow().base_type.to_string() == "ListView" { + if r.borrow().base_type.to_smolstr() == "ListView" { let mut seen_for = false; for se in node.children() { if se.kind() == SyntaxKind::RepeatedElement && !seen_for { @@ -1662,7 +1662,7 @@ impl Element { match lookup_result.property_type { Type::Invalid => { if self.base_type != ElementType::Error { - diag.push_error(if self.base_type.to_string() == "Empty" { + diag.push_error(if self.base_type.to_smolstr() == "Empty" { format!( "Unknown property {unresolved_name}") } else { format!( "Unknown property {unresolved_name} in {}", self.base_type) @@ -1871,7 +1871,7 @@ pub fn type_from_node( let prop_type = tr.lookup_qualified(&qualified_type.members); - if prop_type == Type::Invalid && tr.lookup_element(&qualified_type.to_string()).is_err() { + if prop_type == Type::Invalid && tr.lookup_element(&qualified_type.to_smolstr()).is_err() { diag.push_error(format!("Unknown type '{}'", qualified_type), &qualified_type_node); } else if !prop_type.is_property_type() { diag.push_error( diff --git a/internal/compiler/passes/binding_analysis.rs b/internal/compiler/passes/binding_analysis.rs index ecf8ee43001..c3361f7617e 100644 --- a/internal/compiler/passes/binding_analysis.rs +++ b/internal/compiler/passes/binding_analysis.rs @@ -17,6 +17,8 @@ use crate::namedreference::NamedReference; use crate::object_tree::{find_parent_element, Document, ElementRc, PropertyAnimation}; use derive_more as dm; +use smol_str::ToSmolStr; + /// Maps the alias in the other direction than what the BindingExpression::two_way_binding does. /// So if binding for property A has B in its BindingExpression::two_way_binding, then /// ReverseAliases maps B to A. @@ -478,7 +480,7 @@ fn visit_implicit_layout_info_dependencies( item: &ElementRc, vis: &mut impl FnMut(&PropertyPath, ReadType), ) { - let base_type = item.borrow().base_type.to_string(); + let base_type = item.borrow().base_type.to_smolstr(); const N: ReadType = ReadType::NativeRead; match base_type.as_str() { "Image" => { diff --git a/internal/compiler/passes/lower_property_to_element.rs b/internal/compiler/passes/lower_property_to_element.rs index 45a149917dd..e75bcb5ae6e 100644 --- a/internal/compiler/passes/lower_property_to_element.rs +++ b/internal/compiler/passes/lower_property_to_element.rs @@ -9,7 +9,7 @@ use crate::expression_tree::{BindingExpression, Expression, NamedReference}; use crate::langtype::Type; use crate::object_tree::{self, Component, Element, ElementRc}; use crate::typeregister::TypeRegister; -use smol_str::format_smolstr; +use smol_str::{format_smolstr, ToSmolStr}; use std::rc::Rc; /// If any element in `component` declares a binding to `property_name`, then a new @@ -36,7 +36,7 @@ pub(crate) fn lower_property_to_element( } object_tree::recurse_elem_including_sub_components_no_borrow(component, &(), &mut |elem, _| { - if elem.borrow().base_type.to_string() == element_name { + if elem.borrow().base_type.to_smolstr() == element_name { return; } diff --git a/internal/compiler/passes/unique_id.rs b/internal/compiler/passes/unique_id.rs index 26eb1d97c10..027b20e0dd4 100644 --- a/internal/compiler/passes/unique_id.rs +++ b/internal/compiler/passes/unique_id.rs @@ -30,7 +30,7 @@ fn assign_unique_id_in_component(component: &Rc, count: &mut u32) { let old_id = if !elem_mut.id.is_empty() { elem_mut.id.clone() } else { - elem_mut.base_type.to_string().to_ascii_lowercase().into() + elem_mut.base_type.to_smolstr().to_ascii_lowercase().into() }; elem_mut.id = format_smolstr!("{}-{}", old_id, count); diff --git a/internal/compiler/typeloader.rs b/internal/compiler/typeloader.rs index 188b3cd0b97..64165e9278a 100644 --- a/internal/compiler/typeloader.rs +++ b/internal/compiler/typeloader.rs @@ -1,7 +1,7 @@ // Copyright © SixtyFPS GmbH // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 -use smol_str::SmolStr; +use smol_str::{SmolStr, ToSmolStr}; use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; @@ -59,10 +59,10 @@ impl ImportedName { pub fn from_node(importident: syntax_nodes::ImportIdentifier) -> Self { let external_name = - parser::normalize_identifier(importident.ExternalName().text().to_string().trim()); + parser::normalize_identifier(importident.ExternalName().text().to_smolstr().trim()); let internal_name = match importident.InternalName() { - Some(name_ident) => parser::normalize_identifier(name_ident.text().to_string().trim()), + Some(name_ident) => parser::normalize_identifier(name_ident.text().to_smolstr().trim()), None => external_name.clone(), };