From 841c72d5666c85c19ba23379e9a9fc974da9a6c1 Mon Sep 17 00:00:00 2001 From: Greg Tatum Date: Thu, 1 Oct 2020 15:02:08 -0500 Subject: [PATCH] Favor SmallVec over Box in pluralrules This will help avoid heap allocations as most lists are only a few items long. The only boxed slice that was retained was the SampleRanges, as this can be of an indeterminate length. --- Cargo.lock | 1 + components/plurals/Cargo.toml | 1 + components/plurals/src/rules/ast.rs | 58 +++++++++++++++----------- components/plurals/src/rules/mod.rs | 19 +++++---- components/plurals/src/rules/parser.rs | 16 +++---- 5 files changed, 53 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f48be0d7d4..82bf87c64aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1169,6 +1169,7 @@ dependencies = [ "icu_testdata", "serde", "serde_json", + "smallvec", ] [[package]] diff --git a/components/plurals/Cargo.toml b/components/plurals/Cargo.toml index fa8b0ba8a3e..e86d6d46ddb 100644 --- a/components/plurals/Cargo.toml +++ b/components/plurals/Cargo.toml @@ -37,6 +37,7 @@ icu_provider = { version = "0.3", path = "../../provider/core", features = ["mac icu_locid = { version = "0.3", path = "../locid" } serde = { version = "1.0", default-features = false, features = ["derive", "alloc"], optional = true } displaydoc = { version = "0.2.3", default-features = false } +smallvec = "1.6" [dev-dependencies] criterion = "0.3" diff --git a/components/plurals/src/rules/ast.rs b/components/plurals/src/rules/ast.rs index 468dbc196bd..04d7afac6b7 100644 --- a/components/plurals/src/rules/ast.rs +++ b/components/plurals/src/rules/ast.rs @@ -11,28 +11,29 @@ //! ``` //! use icu::plurals::rules::parse_condition; //! use icu::plurals::rules::ast::*; +//! use smallvec::{SmallVec,smallvec}; //! //! let input = "i = 1"; //! //! let ast = parse_condition(input.as_bytes()) //! .expect("Parsing failed."); //! -//! assert_eq!(ast, Condition(Box::new([ -//! AndCondition(Box::new([ +//! assert_eq!(ast, Condition(smallvec![ +//! AndCondition(smallvec![ //! Relation { //! expression: Expression { //! operand: Operand::I, //! modulus: None, //! }, //! operator: Operator::Eq, -//! range_list: RangeList(Box::new([ +//! range_list: RangeList(smallvec![ //! RangeListItem::Value( //! Value(1) //! ) -//! ])) +//! ]) //! } -//! ])) -//! ]))); +//! ]) +//! ])); //! ``` //! //! [`PluralCategory`]: crate::PluralCategory @@ -41,6 +42,7 @@ use alloc::boxed::Box; use alloc::string::String; use core::ops::RangeInclusive; +use smallvec::SmallVec; /// A complete AST representation of a plural rule. /// Comprises a vector of [`AndConditions`] and optionally a set of [`Samples`]. @@ -98,25 +100,26 @@ pub struct Rule { /// ``` /// use icu::plurals::rules::ast::*; /// use icu::plurals::rules::parse_condition; +/// use smallvec::{SmallVec,smallvec}; /// -/// let condition = Condition(Box::new([ -/// AndCondition(Box::new([Relation { +/// let condition = Condition(smallvec![ +/// AndCondition(smallvec![Relation { /// expression: Expression { /// operand: Operand::I, /// modulus: None, /// }, /// operator: Operator::Eq, -/// range_list: RangeList(Box::new([RangeListItem::Value(Value(5))])), -/// }])), -/// AndCondition(Box::new([Relation { +/// range_list: RangeList(smallvec![RangeListItem::Value(Value(5))]), +/// }]), +/// AndCondition(smallvec![Relation { /// expression: Expression { /// operand: Operand::V, /// modulus: None, /// }, /// operator: Operator::Eq, -/// range_list: RangeList(Box::new([RangeListItem::Value(Value(2))])), -/// }])), -/// ])); +/// range_list: RangeList(smallvec![RangeListItem::Value(Value(2))]), +/// }]), +/// ]); /// /// assert_eq!( /// condition, @@ -127,7 +130,7 @@ pub struct Rule { /// /// [`AndConditions`]: AndCondition #[derive(Debug, Clone, PartialEq)] -pub struct Condition(pub Box<[AndCondition]>); +pub struct Condition(pub SmallVec<[AndCondition; 2]>); /// An incomplete AST representation of a plural rule. Comprises a vector of [`Relations`]. /// @@ -143,16 +146,17 @@ pub struct Condition(pub Box<[AndCondition]>); /// Can be represented by the AST: /// /// ``` -/// use icu::plurals::rules::ast::*; +/// use icu_plurals::rules::ast::*; +/// use smallvec::{SmallVec,smallvec}; /// -/// AndCondition(Box::new([ +/// AndCondition(smallvec![ /// Relation { /// expression: Expression { /// operand: Operand::I, /// modulus: None, /// }, /// operator: Operator::Eq, -/// range_list: RangeList(Box::new([RangeListItem::Value(Value(5))])), +/// range_list: RangeList(smallvec![RangeListItem::Value(Value(5))]), /// }, /// Relation { /// expression: Expression { @@ -160,15 +164,15 @@ pub struct Condition(pub Box<[AndCondition]>); /// modulus: None, /// }, /// operator: Operator::NotEq, -/// range_list: RangeList(Box::new([RangeListItem::Value(Value(2))])), +/// range_list: RangeList(smallvec![RangeListItem::Value(Value(2))]), /// }, -/// ])); +/// ]); /// /// ``` /// /// [`Relations`]: Relation #[derive(Debug, Clone, PartialEq)] -pub struct AndCondition(pub Box<[Relation]>); +pub struct AndCondition(pub SmallVec<[Relation; 2]>); /// An incomplete AST representation of a plural rule. Comprises an [`Expression`], an [`Operator`], and a [`RangeList`]. /// @@ -185,6 +189,7 @@ pub struct AndCondition(pub Box<[Relation]>); /// /// ``` /// use icu::plurals::rules::ast::*; +/// use smallvec::{SmallVec,smallvec}; /// /// Relation { /// expression: Expression { @@ -192,7 +197,7 @@ pub struct AndCondition(pub Box<[Relation]>); /// modulus: None, /// }, /// operator: Operator::Eq, -/// range_list: RangeList(Box::new([RangeListItem::Value(Value(3))])), +/// range_list: RangeList(smallvec![RangeListItem::Value(Value(3))]), /// }; /// /// ``` @@ -304,17 +309,18 @@ pub enum Operand { /// /// ``` /// use icu::plurals::rules::ast::*; +/// use smallvec::{SmallVec,smallvec}; /// -/// RangeList(Box::new([ +/// RangeList(smallvec![ /// RangeListItem::Value(Value(5)), /// RangeListItem::Value(Value(7)), /// RangeListItem::Value(Value(9)), -/// ])); +/// ]); /// ``` /// /// [`RangeListItems`]: RangeListItem #[derive(Debug, Clone, PartialEq)] -pub struct RangeList(pub Box<[RangeListItem]>); +pub struct RangeList(pub SmallVec<[RangeListItem; 2]>); /// An enum of items that appear in a [`RangeList`]: `Range` or a `Value`. /// @@ -408,6 +414,8 @@ pub struct Samples { /// /// ``` /// use icu::plurals::rules::ast::*; +/// use smallvec::{SmallVec,smallvec}; +/// /// SampleList { /// sample_ranges: Box::new([ /// SampleRange { diff --git a/components/plurals/src/rules/mod.rs b/components/plurals/src/rules/mod.rs index 5c65f9e0e80..b074a1ed683 100644 --- a/components/plurals/src/rules/mod.rs +++ b/components/plurals/src/rules/mod.rs @@ -57,24 +57,25 @@ //! ``` //! use icu::plurals::rules::parse_condition; //! use icu::plurals::rules::ast::*; +//! use smallvec::{SmallVec,smallvec}; //! //! let input = "i = 1 and v = 0 @integer 1"; //! //! let ast = parse_condition(input.as_bytes()) //! .expect("Parsing failed."); -//! assert_eq!(ast, Condition(Box::new([ -//! AndCondition(Box::new([ +//! assert_eq!(ast, Condition(smallvec![ +//! AndCondition(smallvec![ //! Relation { //! expression: Expression { //! operand: Operand::I, //! modulus: None, //! }, //! operator: Operator::Eq, -//! range_list: RangeList(Box::new([ +//! range_list: RangeList(smallvec![ //! RangeListItem::Value( //! Value(1) //! ) -//! ])) +//! ]) //! }, //! Relation { //! expression: Expression { @@ -82,14 +83,14 @@ //! modulus: None, //! }, //! operator: Operator::Eq, -//! range_list: RangeList(Box::new([ +//! range_list: RangeList(smallvec![ //! RangeListItem::Value( //! Value(0) //! ) -//! ])) -//! }, -//! ])), -//! ]))); +//! ]) +//! } +//! ]), +//! ])); //! ``` //! //! Finally, we can pass this [`AST`] (in fact, just the [`Condition`] node), diff --git a/components/plurals/src/rules/parser.rs b/components/plurals/src/rules/parser.rs index a2129f0b6bd..5958457d9a5 100644 --- a/components/plurals/src/rules/parser.rs +++ b/components/plurals/src/rules/parser.rs @@ -7,9 +7,9 @@ use super::lexer::{Lexer, Token}; use alloc::string::String; use alloc::string::ToString; use alloc::vec; -use alloc::vec::Vec; use core::iter::Peekable; use displaydoc::Display; +use smallvec::smallvec; #[derive(Display, Debug, PartialEq, Eq)] #[allow(clippy::enum_variant_names)] @@ -118,12 +118,12 @@ impl<'p> Parser<'p> { } fn get_condition(&mut self) -> Result { - let mut result = vec![]; + let mut result = smallvec![]; if let Some(cond) = self.get_and_condition()? { result.push(cond); } else { - return Ok(ast::Condition(result.into_boxed_slice())); + return Ok(ast::Condition(result)); } while self.take_if(Token::Or) { @@ -134,12 +134,12 @@ impl<'p> Parser<'p> { } } // If lexer is not done, error? - Ok(ast::Condition(result.into_boxed_slice())) + Ok(ast::Condition(result)) } fn get_and_condition(&mut self) -> Result, ParserError> { if let Some(relation) = self.get_relation()? { - let mut rel = vec![relation]; + let mut rel = smallvec![relation]; while self.take_if(Token::And) { if let Some(relation) = self.get_relation()? { @@ -148,7 +148,7 @@ impl<'p> Parser<'p> { return Err(ParserError::ExpectedRelation); } } - Ok(Some(ast::AndCondition(rel.into_boxed_slice()))) + Ok(Some(ast::AndCondition(rel))) } else { Ok(None) } @@ -188,14 +188,14 @@ impl<'p> Parser<'p> { } fn get_range_list(&mut self) -> Result { - let mut range_list = Vec::with_capacity(1); + let mut range_list = smallvec![]; loop { range_list.push(self.get_range_list_item()?); if !self.take_if(Token::Comma) { break; } } - Ok(ast::RangeList(range_list.into_boxed_slice())) + Ok(ast::RangeList(range_list)) } fn take_if(&mut self, token: Token) -> bool {