Do not allocate an extra vector unless necessary

This commit is contained in:
Malo 2025-03-31 17:40:19 +02:00
parent 57b3cb6955
commit 2063a9fc93

View File

@ -848,67 +848,106 @@ impl Array {
#[named] #[named]
by: Option<Func>, by: Option<Func>,
) -> SourceResult<Array> { ) -> SourceResult<Array> {
let mut are_in_order = |mut x: Value, mut y: Value| { match by {
if let Some(f) = &key { Some(by) => {
x = f.call(engine, context, [x])?; let mut are_in_order = |mut x, mut y| {
y = f.call(engine, context, [y])?; if let Some(f) = &key {
} // We rely on `comemo`'s memoization of function
match &by { // evaluation to not excessively reevaluate the key.
Some(f) => match f.call(engine, context, [x, y])? { x = f.call(engine, context, [x])?;
Value::Bool(b) => Ok(b), y = f.call(engine, context, [y])?;
x => {
bail!(span, "expected boolean from `by` function, got {}", x.ty())
} }
}, match by.call(engine, context, [x, y])? {
None => { Value::Bool(b) => Ok(b),
// `x` and `y` are in order iff `y` is not strictly greater x => {
// than `y`. bail!(
ops::compare(&x, &y).at(span).map(|o| o != Ordering::Greater) span,
} "expected boolean from `by` function, got {}",
} x.ty(),
}; )
let mut vec = self.0.into_iter().enumerate().collect::<Vec<_>>();
let mut result = Ok(());
// We use `glidesort` instead of the standard library sorting algorithm
// to prevent panics (see https://github.com/typst/typst/pull/5627).
glidesort::sort_by(&mut vec, |(i, x), (j, y)| {
// Because we use booleans for the comparison function, in order to
// keep the sort stable, we need to compare in the right order.
if i < j {
// If `x` and `y` appear in this order in the original array,
// then we should change their order (i.e., return
// `Ordering::Greater`) iff `y` is strictly less than `x` (i.e.,
// `compare(x, y)` returns `false`). Otherwise, we should keep
// them in the same order (i.e., return `Ordering::Less`).
match are_in_order(x.clone(), y.clone()) {
Ok(false) => Ordering::Greater,
Ok(true) => Ordering::Less,
Err(err) => {
if result.is_ok() {
result = Err(err);
} }
Ordering::Equal
} }
} };
} else { // If a comparison function is provided, we use `glidesort`
// If `x` and `y` appear in the opposite order in the original // instead of the standard library sorting algorithm to prevent
// array, then we should change their order (i.e., return // panics in case the comparison function does not define a
// `Ordering::Less`) iff `x` is strictly less than `y` (i.e., // valid order (see https://github.com/typst/typst/pull/5627).
// `compare(y, x)` returns `false`). Otherwise, we should keep let mut result = Ok(());
// them in the same order (i.e., return `Ordering::Less`). let mut vec = self.0.into_iter().enumerate().collect::<Vec<_>>();
match are_in_order(y.clone(), x.clone()) { glidesort::sort_by(&mut vec, |(i, x), (j, y)| {
Ok(false) => Ordering::Less, // Because we use booleans for the comparison function, in
Ok(true) => Ordering::Greater, // order to keep the sort stable, we need to compare in the
Err(err) => { // right order.
if result.is_ok() { if i < j {
result = Err(err); // If `x` and `y` appear in this order in the original
// array, then we should change their order (i.e.,
// return `Ordering::Greater`) iff `y` is strictly less
// than `x` (i.e., `compare(x, y)` returns `false`).
// Otherwise, we should keep them in the same order
// (i.e., return `Ordering::Less`).
match are_in_order(x.clone(), y.clone()) {
Ok(false) => Ordering::Greater,
Ok(true) => Ordering::Less,
Err(err) => {
if result.is_ok() {
result = Err(err);
}
Ordering::Equal
}
}
} else {
// If `x` and `y` appear in the opposite order in the
// original array, then we should change their order
// (i.e., return `Ordering::Less`) iff `x` is strictly
// less than `y` (i.e., `compare(y, x)` returns
// `false`). Otherwise, we should keep them in the same
// order (i.e., return `Ordering::Less`).
match are_in_order(y.clone(), x.clone()) {
Ok(false) => Ordering::Less,
Ok(true) => Ordering::Greater,
Err(err) => {
if result.is_ok() {
result = Err(err);
}
Ordering::Equal
}
} }
Ordering::Equal
} }
} });
result.map(|()| vec.into_iter().map(|(_, x)| x).collect())
} }
});
result.map(|_| vec.into_iter().map(|(_, x)| x).collect()) None => {
let mut key_of = |x: Value| match &key {
// We rely on `comemo`'s memoization of function evaluation
// to not excessively reevaluate the key.
Some(f) => f.call(engine, context, [x]),
None => Ok(x),
};
// If no comparison function is provided, we know the order is
// valid, so we can use the standard library sort and prevent an
// extra allocation.
let mut result = Ok(());
let mut vec = self.0;
vec.make_mut().sort_by(|a, b| {
match (key_of(a.clone()), key_of(b.clone())) {
(Ok(a), Ok(b)) => ops::compare(&a, &b).unwrap_or_else(|err| {
if result.is_ok() {
result = Err(err).at(span);
}
Ordering::Equal
}),
(Err(e), _) | (_, Err(e)) => {
if result.is_ok() {
result = Err(e);
}
Ordering::Equal
}
}
});
result.map(|()| vec.into())
}
}
} }
/// Deduplicates all items in the array. /// Deduplicates all items in the array.