Skip to content

Commit

Permalink
perf: Avoid allocating a String for each formatted operator (#4721)
Browse files Browse the repository at this point in the history 
* perf: Avoid allocating a String for each formatted operator

Refactors the parse/display of operators to use a common macro which removes duplication
and ensures that we cover all variants. I changed `ToString for LogicalOperator` to a `fmt::Display`
implementation as there is a default implementation that auto implements `ToString` (`ToString for T where T: fmt::Display`).

* chore: make generate
  • Loading branch information
Markus Westerlind authored May 10, 2022
1 parent d2753ff commit 18f3d8e
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 75 deletions.
134 changes: 65 additions & 69 deletions libflux/flux-core/src/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ pub mod walk;
use std::{collections::HashMap, fmt, str::FromStr, vec::Vec};

use chrono::FixedOffset;
use derive_more::Display;
use serde::{
de::{Deserialize, Deserializer, Error, Visitor},
ser::{Serialize, SerializeSeq, Serializer},
Expand Down Expand Up @@ -985,55 +984,40 @@ pub struct FunctionExpr {
/// based on whether the comparison is true.
/// Arithmetic operators take numerical values (either literals or variables)
/// as their operands and return a single numerical value.
#[derive(Debug, Display, PartialEq, Clone)]
#[derive(Debug, PartialEq, Clone)]
#[allow(missing_docs)]
pub enum Operator {
#[display(fmt = "*")]
MultiplicationOperator,
#[display(fmt = "/")]
DivisionOperator,
#[display(fmt = "%")]
ModuloOperator,
#[display(fmt = "^")]
PowerOperator,
#[display(fmt = "+")]
AdditionOperator,
#[display(fmt = "-")]
SubtractionOperator,
#[display(fmt = "<=")]
LessThanEqualOperator,
#[display(fmt = "<")]
LessThanOperator,
#[display(fmt = ">=")]
GreaterThanEqualOperator,
#[display(fmt = ">")]
GreaterThanOperator,
#[display(fmt = "startswith")]
StartsWithOperator,
#[display(fmt = "in")]
InOperator,
#[display(fmt = "not")]
NotOperator,
#[display(fmt = "exists")]
ExistsOperator,
#[display(fmt = "not empty")]
NotEmptyOperator,
#[display(fmt = "empty")]
EmptyOperator,
#[display(fmt = "==")]
EqualOperator,
#[display(fmt = "!=")]
NotEqualOperator,
#[display(fmt = "=~")]
RegexpMatchOperator,
#[display(fmt = "!~")]
NotRegexpMatchOperator,

// this is necessary for bad binary expressions.
#[display(fmt = "<INVALID_OP>")]
InvalidOperator,
}

impl fmt::Display for Operator {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(self.as_str())
}
}

impl Serialize for Operator {
fn serialize<S>(&self, serializer: S) -> Result<<S as Serializer>::Ok, <S as Serializer>::Error>
where
Expand All @@ -1043,35 +1027,56 @@ impl Serialize for Operator {
}
}

impl FromStr for Operator {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"*" => Ok(Operator::MultiplicationOperator),
"/" => Ok(Operator::DivisionOperator),
"%" => Ok(Operator::ModuloOperator),
"^" => Ok(Operator::PowerOperator),
"+" => Ok(Operator::AdditionOperator),
"-" => Ok(Operator::SubtractionOperator),
"<=" => Ok(Operator::LessThanEqualOperator),
"<" => Ok(Operator::LessThanOperator),
">=" => Ok(Operator::GreaterThanEqualOperator),
">" => Ok(Operator::GreaterThanOperator),
"startswith" => Ok(Operator::StartsWithOperator),
"in" => Ok(Operator::InOperator),
"not" => Ok(Operator::NotOperator),
"exists" => Ok(Operator::ExistsOperator),
"not empty" => Ok(Operator::NotEmptyOperator),
"empty" => Ok(Operator::EmptyOperator),
"==" => Ok(Operator::EqualOperator),
"!=" => Ok(Operator::NotEqualOperator),
"=~" => Ok(Operator::RegexpMatchOperator),
"!~" => Ok(Operator::NotRegexpMatchOperator),
"<INVALID_OP>" => Ok(Operator::InvalidOperator),
_ => Err(format!("unknown operator: {}", s)),
macro_rules! from_to_str {
($name: ident, $($str: tt => $op: tt),* $(,)?) => {
impl FromStr for $name {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(match s {
$(
$str => $name :: $op,
)*
_ => return Err(format!("unknown operator: {}", s)),
})
}
}
}

impl $name {
pub(crate) fn as_str(&self) -> &'static str {
match self {
$(
$name :: $op => $str,
)*
}
}
}
};
}

from_to_str! {
Operator,
"*" => MultiplicationOperator,
"/" => DivisionOperator,
"%" => ModuloOperator,
"^" => PowerOperator,
"+" => AdditionOperator,
"-" => SubtractionOperator,
"<=" => LessThanEqualOperator,
"<" => LessThanOperator,
">=" => GreaterThanEqualOperator,
">" => GreaterThanOperator,
"startswith" => StartsWithOperator,
"in" => InOperator,
"not" => NotOperator,
"exists" => ExistsOperator,
"not empty" => NotEmptyOperator,
"empty" => EmptyOperator,
"==" => EqualOperator,
"!=" => NotEqualOperator,
"=~" => RegexpMatchOperator,
"!~" => NotRegexpMatchOperator,
"<INVALID_OP>" => InvalidOperator,
}

struct OperatorVisitor;
Expand Down Expand Up @@ -1140,12 +1145,15 @@ pub enum LogicalOperator {
OrOperator,
}

impl ToString for LogicalOperator {
fn to_string(&self) -> String {
match self {
LogicalOperator::AndOperator => "and".to_string(),
LogicalOperator::OrOperator => "or".to_string(),
}
from_to_str! {
LogicalOperator,
"and" => AndOperator,
"or" => OrOperator,
}

impl fmt::Display for LogicalOperator {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(self.as_str())
}
}

Expand All @@ -1158,18 +1166,6 @@ impl Serialize for LogicalOperator {
}
}

impl FromStr for LogicalOperator {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"and" => Ok(LogicalOperator::AndOperator),
"or" => Ok(LogicalOperator::OrOperator),
_ => Err(format!("unknown logical operator: {}", s)),
}
}
}

struct LogicalOperatorVisitor;

impl<'de> Visitor<'de> for LogicalOperatorVisitor {
Expand Down
8 changes: 4 additions & 4 deletions libflux/flux-core/src/formatter/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,14 +1226,14 @@ impl<'doc> Formatter<'doc> {
ast::Expression::Logical(expr) => self.format_binary_expression(
parent,
&expr.left,
expr.operator.to_string(),
expr.operator.as_str(),
&expr.right,
),
ast::Expression::Member(n) => self.format_member_expression(n),
ast::Expression::Binary(expr) => self.format_binary_expression(
parent,
&expr.left,
expr.operator.to_string(),
expr.operator.as_str(),
&expr.right,
),
ast::Expression::Unary(n) => {
Expand Down Expand Up @@ -1499,7 +1499,7 @@ impl<'doc> Formatter<'doc> {
&mut self,
mut parent: &'doc ast::Expression,
lhs: &'doc ast::Expression,
mut operator: String,
mut operator: &'doc str,
mut rhs: &'doc ast::Expression,
) -> Doc<'doc> {
let arena = self.arena;
Expand All @@ -1526,7 +1526,7 @@ impl<'doc> Formatter<'doc> {
];

parent = rhs;
operator = expr.operator.to_string();
operator = expr.operator.as_str();
rhs = &expr.right;
}
_ => {
Expand Down
4 changes: 2 additions & 2 deletions libflux/go/libflux/buildinfo.gen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ var sourceHashes = map[string]string{
"libflux/Cargo.toml": "91ac4e8b467440c6e8a9438011de0e7b78c2732403bb067d4dd31539ac8a90c1",
"libflux/flux-core/Cargo.toml": "2290ce95d56e0f514b7c8285693a8011cddfc652bf2aad24edfeb43911f57ba9",
"libflux/flux-core/src/ast/check/mod.rs": "47e06631f249715a44c9c8fa897faf142ad0fa26f67f8cfd5cd201e82cb1afc8",
"libflux/flux-core/src/ast/mod.rs": "87555cc82f6adb1606c3c6dea4ef91a3b0763b6a64eb2dfbc682bb67f4957d9c",
"libflux/flux-core/src/ast/mod.rs": "c1296bae3a67fbf5b4b5d4f5d93933e7c013a7b9a79f2d028cc9cc93f34b41c5",
"libflux/flux-core/src/ast/walk/mod.rs": "aa75319f33938c43db4b907249bc67508f00fba02e45ae175e939e96e178298b",
"libflux/flux-core/src/bin/README.md": "c1245a4938c923d065647b4dc4f7e19486e85c93d868ef2f7f47ddff62ec81df",
"libflux/flux-core/src/bin/fluxc.rs": "bf275289e690236988049fc0a07cf832dbac25bb5739c02135b069dcdfab4d0f",
"libflux/flux-core/src/bin/fluxdoc.rs": "354a8dfd262f223412ee491a9947962f993e43c5467f8ee37c4f15528dc5b571",
"libflux/flux-core/src/doc/example.rs": "6414756b3c74df1b58fdb739592e74ded3f89d85d647809333f72a3f6aad146f",
"libflux/flux-core/src/doc/mod.rs": "e8aae31bc4a60836d7258a03f6827b76e9fad44c889db6a21d6679c26818f2d2",
"libflux/flux-core/src/errors.rs": "10e131532c3d40062dd3e2445e58d0c05370b64383892a80a056dfac5fcd49b9",
"libflux/flux-core/src/formatter/mod.rs": "2b37befce1624318d8eafd1547881d342daa3cb66bf12967689006ffd2eafaf3",
"libflux/flux-core/src/formatter/mod.rs": "de8e19e431a221840a94765d1d3bc589d0d67724d508877c81774c05beb58bd2",
"libflux/flux-core/src/lib.rs": "443aed16dd600eecc1ffbee2d2dead6788e796cd5a278eb5dafb123843b8959e",
"libflux/flux-core/src/map.rs": "342c1cc111d343f01b97f38be10a9f1097bdd57cdc56f55e92fd3ed5028e6973",
"libflux/flux-core/src/parser/mod.rs": "f113ded73ee5b9ed920082f45044ae0d33e42c34b3deb717375069b8373d7f02",
Expand Down

0 comments on commit 18f3d8e

Please sign in to comment.