extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-08-09 15:06:03 +02:00
Code Issues Packages Projects Releases Wiki Activity

Shrink the size of Expr (#12610)

Browse Source 
# Description
Continuing from #12568, this PR further reduces the size of `Expr` from
64 to 40 bytes. It also reduces `Expression` from 128 to 96 bytes and
`Type` from 32 to 24 bytes.

This was accomplished by:
- for `Expr` with multiple fields (e.g., `Expr::Thing(A, B, C)`),
merging the fields into new AST struct types and then boxing this struct
(e.g. `Expr::Thing(Box<ABC>)`).
- replacing `Vec<T>` with `Box<[T]>` in multiple places. `Expr`s and
`Expression`s should rarely be mutated, if at all, so this optimization
makes sense.

By reducing the size of these types, I didn't notice a large performance
improvement (at least compared to #12568). But this PR does reduce the
memory usage of nushell. My config is somewhat light so I only noticed a
difference of 1.4MiB (38.9MiB vs 37.5MiB).

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
This commit is contained in:
Ian Manske
2024-04-24 15:46:35 +00:00
committed by GitHub
parent c52884b3c8
commit 9996e4a1f8
195 changed files with 688 additions and 601 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
crates/nu-protocol/src/ast/expr.rs
View File

@ -2,13 +2,10 @@ use chrono::FixedOffset;
use serde::{Deserialize, Serialize};
use super::{
Call, CellPath, Expression, ExternalArgument, FullCellPath, MatchPattern, Operator,
RangeOperator,
};
use crate::{
ast::ImportPattern, ast::Unit, engine::EngineState, BlockId, OutDest, Signature, Span, Spanned,
VarId,
Call, CellPath, Expression, ExternalArgument, FullCellPath, Keyword, MatchPattern, Operator,
Range, Table, ValueWithUnit,
};
use crate::{ast::ImportPattern, engine::EngineState, BlockId, OutDest, Signature, Span, VarId};
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum Expr {
@ -16,16 +13,11 @@ pub enum Expr {
Int(i64),
Float(f64),
Binary(Vec<u8>),
Range(
Option<Box<Expression>>, // from
Option<Box<Expression>>, // next value after "from"
Option<Box<Expression>>, // to
RangeOperator,
),
Range(Box<Range>),
Var(VarId),
VarDecl(VarId),
Call(Box<Call>),
ExternalCall(Box<Expression>, Vec<ExternalArgument>), // head, args
ExternalCall(Box<Expression>, Box<[ExternalArgument]>), // head, args
Operator(Operator),
RowCondition(BlockId),
UnaryNot(Box<Expression>),
@ -35,10 +27,10 @@ pub enum Expr {
Closure(BlockId),
MatchBlock(Vec<(MatchPattern, Expression)>),
List(Vec<ListItem>),
Table(Vec<Expression>, Vec<Vec<Expression>>),
Table(Table),
Record(Vec<RecordItem>),
Keyword(Vec<u8>, Span, Box<Expression>),
ValueWithUnit(Box<Expression>, Spanned<Unit>),
Keyword(Box<Keyword>),
ValueWithUnit(Box<ValueWithUnit>),
DateTime(chrono::DateTime<FixedOffset>),
Filepath(String, bool),
Directory(String, bool),
@ -54,6 +46,11 @@ pub enum Expr {
Garbage,
}
// This is to document/enforce the size of `Expr` in bytes.
// We should try to avoid increasing the size of `Expr`,
// and PRs that do so will have to change the number below so that it's noted in review.
const _: () = assert!(std::mem::size_of::<Expr>() <= 40);
impl Expr {
pub fn pipe_redirection(
&self,
@ -72,15 +69,15 @@ impl Expr {
| Expr::Int(_)
| Expr::Float(_)
| Expr::Binary(_)
| Expr::Range(_, _, _, _)
| Expr::Range(_)
| Expr::Var(_)
| Expr::UnaryNot(_)
| Expr::BinaryOp(_, _, _)
| Expr::Closure(_) // piping into a closure value, not into a closure call
| Expr::List(_)
| Expr::Table(_, _)
| Expr::Table(_)
| Expr::Record(_)
| Expr::ValueWithUnit(_, _)
| Expr::ValueWithUnit(_)
| Expr::DateTime(_)
| Expr::String(_)
| Expr::CellPath(_)
@ -112,7 +109,7 @@ impl Expr {
// No override necessary, pipes will always be created in eval
(None, None)
}
Expr::Keyword(_, _, _) => {
Expr::Keyword(_) => {
// Not sure about this; let's return no redirection override for now.
(None, None)
}

46
crates/nu-protocol/src/ast/expression.rs
View File

@ -88,7 +88,7 @@ impl Expression {
pub fn as_keyword(&self) -> Option<&Expression> {
match &self.expr {
Expr::Keyword(_, _, expr) => Some(expr),
Expr::Keyword(kw) => Some(&kw.expr),
_ => None,
}
}
@ -189,7 +189,9 @@ impl Expression {
if head.has_in_variable(working_set) {
return true;
}
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in args {
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in
args.as_ref()
{
if expr.has_in_variable(working_set) {
return true;
}
@ -211,7 +213,7 @@ impl Expression {
Expr::Nothing => false,
Expr::GlobPattern(_, _) => false,
Expr::Int(_) => false,
Expr::Keyword(_, _, expr) => expr.has_in_variable(working_set),
Expr::Keyword(kw) => kw.expr.has_in_variable(working_set),
Expr::List(list) => {
for item in list {
if item.expr().has_in_variable(working_set) {
@ -230,18 +232,18 @@ impl Expression {
}
Expr::Operator(_) => false,
Expr::MatchBlock(_) => false,
Expr::Range(left, middle, right, ..) => {
if let Some(left) = &left {
Expr::Range(range) => {
if let Some(left) = &range.from {
if left.has_in_variable(working_set) {
return true;
}
}
if let Some(middle) = &middle {
if let Some(middle) = &range.next {
if middle.has_in_variable(working_set) {
return true;
}
}
if let Some(right) = &right {
if let Some(right) = &range.to {
if right.has_in_variable(working_set) {
return true;
}
@ -283,14 +285,14 @@ impl Expression {
false
}
}
Expr::Table(headers, cells) => {
for header in headers {
Expr::Table(table) => {
for header in table.columns.as_ref() {
if header.has_in_variable(working_set) {
return true;
}
}
for row in cells {
for row in table.rows.as_ref() {
for cell in row.iter() {
if cell.has_in_variable(working_set) {
return true;
@ -301,7 +303,7 @@ impl Expression {
false
}
Expr::ValueWithUnit(expr, _) => expr.has_in_variable(working_set),
Expr::ValueWithUnit(value) => value.expr.has_in_variable(working_set),
Expr::Var(var_id) => *var_id == IN_VARIABLE_ID,
Expr::VarDecl(_) => false,
}
@ -372,7 +374,9 @@ impl Expression {
Expr::DateTime(_) => {}
Expr::ExternalCall(head, args) => {
head.replace_span(working_set, replaced, new_span);
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in args {
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in
args.as_mut()
{
expr.replace_span(working_set, replaced, new_span);
}
}
@ -391,7 +395,7 @@ impl Expression {
Expr::GlobPattern(_, _) => {}
Expr::MatchBlock(_) => {}
Expr::Int(_) => {}
Expr::Keyword(_, _, expr) => expr.replace_span(working_set, replaced, new_span),
Expr::Keyword(kw) => kw.expr.replace_span(working_set, replaced, new_span),
Expr::List(list) => {
for item in list {
item.expr_mut()
@ -399,14 +403,14 @@ impl Expression {
}
}
Expr::Operator(_) => {}
Expr::Range(left, middle, right, ..) => {
if let Some(left) = left {
Expr::Range(range) => {
if let Some(left) = &mut range.from {
left.replace_span(working_set, replaced, new_span)
}
if let Some(middle) = middle {
if let Some(middle) = &mut range.next {
middle.replace_span(working_set, replaced, new_span)
}
if let Some(right) = right {
if let Some(right) = &mut range.to {
right.replace_span(working_set, replaced, new_span)
}
}
@ -441,19 +445,19 @@ impl Expression {
*block_id = working_set.add_block(Arc::new(block));
}
Expr::Table(headers, cells) => {
for header in headers {
Expr::Table(table) => {
for header in table.columns.as_mut() {
header.replace_span(working_set, replaced, new_span)
}
for row in cells {
for row in table.rows.as_mut() {
for cell in row.iter_mut() {
cell.replace_span(working_set, replaced, new_span)
}
}
}
Expr::ValueWithUnit(expr, _) => expr.replace_span(working_set, replaced, new_span),
Expr::ValueWithUnit(value) => value.expr.replace_span(working_set, replaced, new_span),
Expr::Var(_) => {}
Expr::VarDecl(_) => {}
}

10
crates/nu-protocol/src/ast/keyword.rs Normal file
View File

@ -0,0 +1,10 @@
use super::Expression;
use crate::Span;
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Keyword {
pub keyword: Box<[u8]>,
pub span: Span,
pub expr: Expression,
}

8
crates/nu-protocol/src/ast/mod.rs
View File

@ -4,10 +4,14 @@ mod cell_path;
mod expr;
mod expression;
mod import_pattern;
mod keyword;
mod match_pattern;
mod operator;
mod pipeline;
mod range;
pub mod table;
mod unit;
mod value_with_unit;
pub use block::*;
pub use call::*;
@ -15,7 +19,11 @@ pub use cell_path::*;
pub use expr::*;
pub use expression::*;
pub use import_pattern::*;
pub use keyword::*;
pub use match_pattern::*;
pub use operator::*;
pub use pipeline::*;
pub use range::*;
pub use table::Table;
pub use unit::*;
pub use value_with_unit::*;

10
crates/nu-protocol/src/ast/range.rs Normal file
View File

@ -0,0 +1,10 @@
use super::{Expression, RangeOperator};
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Range {
pub from: Option<Expression>,
pub next: Option<Expression>,
pub to: Option<Expression>,
pub operator: RangeOperator,
}

8
crates/nu-protocol/src/ast/table.rs Normal file
View File

@ -0,0 +1,8 @@
use super::Expression;
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Table {
pub columns: Box<[Expression]>,
pub rows: Box<[Box<[Expression]>]>,
}

9
crates/nu-protocol/src/ast/value_with_unit.rs Normal file
View File

@ -0,0 +1,9 @@
use super::Expression;
use crate::{Spanned, Unit};
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ValueWithUnit {
pub expr: Expression,
pub unit: Spanned<Unit>,
}

8
crates/nu-protocol/src/debugger/profiler.rs
View File

@ -243,22 +243,22 @@ fn expr_to_string(engine_state: &EngineState, expr: &Expr) -> String {
Expr::GlobPattern(_, _) => "glob pattern".to_string(),
Expr::ImportPattern(_) => "import pattern".to_string(),
Expr::Int(_) => "int".to_string(),
Expr::Keyword(_, _, _) => "keyword".to_string(),
Expr::Keyword(_) => "keyword".to_string(),
Expr::List(_) => "list".to_string(),
Expr::MatchBlock(_) => "match block".to_string(),
Expr::Nothing => "nothing".to_string(),
Expr::Operator(_) => "operator".to_string(),
Expr::Overlay(_) => "overlay".to_string(),
Expr::Range(_, _, _, _) => "range".to_string(),
Expr::Range(_) => "range".to_string(),
Expr::Record(_) => "record".to_string(),
Expr::RowCondition(_) => "row condition".to_string(),
Expr::Signature(_) => "signature".to_string(),
Expr::String(_) => "string".to_string(),
Expr::StringInterpolation(_) => "string interpolation".to_string(),
Expr::Subexpression(_) => "subexpression".to_string(),
Expr::Table(_, _) => "table".to_string(),
Expr::Table(_) => "table".to_string(),
Expr::UnaryNot(_) => "unary not".to_string(),
Expr::ValueWithUnit(_, _) => "value with unit".to_string(),
Expr::ValueWithUnit(_) => "value with unit".to_string(),
Expr::Var(_) => "var".to_string(),
Expr::VarDecl(_) => "var decl".to_string(),
}

18
crates/nu-protocol/src/engine/pattern_match.rs
View File

@ -141,11 +141,11 @@ impl Matcher for Pattern {
false
}
}
Expr::ValueWithUnit(amount, unit) => {
let span = unit.span;
Expr::ValueWithUnit(val) => {
let span = val.unit.span;
if let Expr::Int(size) = amount.expr {
match &unit.item.build_value(size, span) {
if let Expr::Int(size) = val.expr.expr {
match &val.unit.item.build_value(size, span) {
Ok(v) => v == value,
_ => false,
}
@ -153,10 +153,10 @@ impl Matcher for Pattern {
false
}
}
Expr::Range(start, step, end, inclusion) => {
Expr::Range(range) => {
// TODO: Add support for floats
let start = if let Some(start) = &start {
let start = if let Some(start) = &range.from {
match &start.expr {
Expr::Int(start) => *start,
_ => return false,
@ -165,7 +165,7 @@ impl Matcher for Pattern {
0
};
let end = if let Some(end) = &end {
let end = if let Some(end) = &range.to {
match &end.expr {
Expr::Int(end) => *end,
_ => return false,
@ -174,7 +174,7 @@ impl Matcher for Pattern {
i64::MAX
};
let step = if let Some(step) = step {
let step = if let Some(step) = &range.next {
match &step.expr {
Expr::Int(step) => *step - start,
_ => return false,
@ -192,7 +192,7 @@ impl Matcher for Pattern {
};
if let Value::Int { val, .. } = &value {
if matches!(inclusion.inclusion, RangeInclusion::RightExclusive) {
if matches!(range.operator.inclusion, RangeInclusion::RightExclusive) {
*val >= start && *val < end && ((*val - start) % step) == 0
} else {
*val >= start && *val <= end && ((*val - start) % step) == 0

28
crates/nu-protocol/src/eval_base.rs
View File

@ -100,9 +100,9 @@ pub trait Eval {
Ok(Value::record(record, expr.span))
}
Expr::Table(headers, vals) => {
Expr::Table(table) => {
let mut output_headers = vec![];
for expr in headers {
for expr in table.columns.as_ref() {
let header = Self::eval::<D>(state, mut_state, expr)?.coerce_into_string()?;
if let Some(idx) = output_headers
.iter()
@ -111,7 +111,7 @@ pub trait Eval {
return Err(ShellError::ColumnDefinedTwice {
col_name: header,
second_use: expr.span,
first_use: headers[idx].span,
first_use: table.columns[idx].span,
});
} else {
output_headers.push(header);
@ -119,8 +119,8 @@ pub trait Eval {
}
let mut output_rows = vec![];
for val in vals {
let record = output_headers.iter().zip(val).map(|(col, expr)| {
for val in table.rows.as_ref() {
let record = output_headers.iter().zip(val.as_ref()).map(|(col, expr)| {
Self::eval::<D>(state, mut_state, expr).map(|val| (col.clone(), val))
}).collect::<Result<_,_>>()?;
@ -131,15 +131,15 @@ pub trait Eval {
}
Ok(Value::list(output_rows, expr.span))
}
Expr::Keyword(_, _, expr) => Self::eval::<D>(state, mut_state, expr),
Expr::Keyword(kw) => Self::eval::<D>(state, mut_state, &kw.expr),
Expr::String(s) => Ok(Value::string(s.clone(), expr.span)),
Expr::Nothing => Ok(Value::nothing(expr.span)),
Expr::ValueWithUnit(e, unit) => match Self::eval::<D>(state, mut_state, e)? {
Value::Int { val, .. } => unit.item.build_value(val, unit.span),
Expr::ValueWithUnit(value) => match Self::eval::<D>(state, mut_state, &value.expr)? {
Value::Int { val, .. } => value.unit.item.build_value(val, value.unit.span),
x => Err(ShellError::CantConvert {
to_type: "unit value".into(),
from_type: x.get_type().to_string(),
span: e.span,
span: value.expr.span,
help: None,
}),
},
@ -150,27 +150,27 @@ pub trait Eval {
Expr::Subexpression(block_id) => {
Self::eval_subexpression::<D>(state, mut_state, *block_id, expr.span)
}
Expr::Range(from, next, to, operator) => {
let from = if let Some(f) = from {
Expr::Range(range) => {
let from = if let Some(f) = &range.from {
Self::eval::<D>(state, mut_state, f)?
} else {
Value::nothing(expr.span)
};
let next = if let Some(s) = next {
let next = if let Some(s) = &range.next {
Self::eval::<D>(state, mut_state, s)?
} else {
Value::nothing(expr.span)
};
let to = if let Some(t) = to {
let to = if let Some(t) = &range.to {
Self::eval::<D>(state, mut_state, t)?
} else {
Value::nothing(expr.span)
};
Ok(Value::range(
Range::new(from, next, to, operator.inclusion, expr.span)?,
Range::new(from, next, to, range.operator.inclusion, expr.span)?,
expr.span,
))
}

14
crates/nu-protocol/src/ty.rs
View File

@ -15,7 +15,7 @@ pub enum Type {
Bool,
CellPath,
Closure,
Custom(String),
Custom(Box<str>),
Date,
Duration,
Error,
@ -28,14 +28,22 @@ pub enum Type {
Nothing,
Number,
Range,
Record(Vec<(String, Type)>),
Record(Box<[(String, Type)]>),
Signature,
String,
Glob,
Table(Vec<(String, Type)>),
Table(Box<[(String, Type)]>),
}
impl Type {
pub fn record() -> Self {
Self::Record([].into())
}
pub fn table() -> Self {
Self::Table([].into())
}
pub fn is_subtype(&self, other: &Type) -> bool {
// Structural subtyping
let is_subtype_collection = |this: &[(String, Type)], that: &[(String, Type)]| {

2
crates/nu-protocol/src/value/mod.rs
View File

@ -793,7 +793,7 @@ impl Value {
Value::Error { .. } => Type::Error,
Value::Binary { .. } => Type::Binary,
Value::CellPath { .. } => Type::CellPath,
Value::Custom { val, .. } => Type::Custom(val.type_name()),
Value::Custom { val, .. } => Type::Custom(val.type_name().into()),
}
}