Skip to content

Commit

Permalink
Speed up the comparison time lookups
Browse files Browse the repository at this point in the history
We use a Vec here now because a HashMap would require hashing the
comparison and then comparing the comparison with the string at the
index calculated from the hash. This means at least two full iterations
over the string are necessary, with one of them being somewhat expensive
due to the hashing. Most of the time it is faster to just iterate the
few comparisons we have and compare them directly. Most will be rejected
right away as the first byte doesn't even match, so in the end you'll
end up with less than two full iterations over the string. In fact most
of the time Personal Best will be the first in the list and that's the
one we most often want to look up anyway.

One additional reason for doing this is that the ahash that was
calculated for the HashMap uses 128-bit multiplications which regressed
a lot in Rust 1.44 for targets where the `compiler-builtins` helpers
were used. rust-lang/rust#73135

We could potentially look into interning our comparisons in the future
which could yield even better performance.
  • Loading branch information
CryZe committed Jun 10, 2020
1 parent 73cc3d1 commit e1dbd59
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 16 deletions.
83 changes: 83 additions & 0 deletions src/run/comparisons.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
use crate::platform::prelude::*;
use crate::Time;

// We use a Vec here because a HashMap would require hashing the comparison and
// then comparing the comparison with the string at the index calculated from
// the hash. This means at least two full iterations over the string are
// necessary, with one of them being somewhat expensive due to the hashing. Most
// of the time it is faster to just iterate the few comparisons we have and
// compare them directly. Most will be rejected right away as the first byte
// doesn't even match, so in the end you'll end up with less than two full
// iterations over the string. In fact most of the time Personal Best will be
// the first in the list and that's the one we most often want to look up
// anyway.
//
// One additional reason for doing this is that the ahash that was calculated
// for the HashMap uses 128-bit multiplications which regressed a lot in Rust
// 1.44 for targets where the `compiler-builtins` helpers were used.
// https://github.com/rust-lang/rust/issues/73135
//
// We could potentially look into interning our comparisons in the future which
// could yield even better performance.

/// A collection of a segment's comparison times.
#[derive(Clone, Default, Debug, PartialEq)]
pub struct Comparisons(Vec<(Box<str>, Time)>);

impl Comparisons {
fn index_of(&self, comparison: &str) -> Option<usize> {
Some(
self.0
.iter()
.enumerate()
.find(|(_, (c, _))| &**c == comparison)?
.0,
)
}

/// Accesses the time for the comparison specified.
pub fn get(&self, comparison: &str) -> Option<Time> {
Some(self.0[self.index_of(comparison)?].1)
}

/// Accesses the time for the comparison specified, or inserts a new empty
/// one if there is none.
pub fn get_or_insert_default(&mut self, comparison: &str) -> &mut Time {
if let Some(index) = self.index_of(comparison) {
&mut self.0[index].1
} else {
self.0.push((comparison.into(), Time::default()));
&mut self.0.last_mut().unwrap().1
}
}

/// Sets the time for the comparison specified.
pub fn set(&mut self, comparison: &str, time: Time) {
*self.get_or_insert_default(comparison) = time;
}

/// Removes the time for the comparison specified and returns it if there
/// was one.
pub fn remove(&mut self, comparison: &str) -> Option<Time> {
let index = self.index_of(comparison)?;
let (_, time) = self.0.remove(index);
Some(time)
}

/// Clears all the comparisons and their times.
pub fn clear(&mut self) {
self.0.clear();
}

/// Iterates over all the comparisons and their times.
pub fn iter(&self) -> impl Iterator<Item = &(Box<str>, Time)> + '_ {
self.0.iter()
}

/// Mutably iterates over all the comparisons and their times. Be careful
/// when modifying the comparison name. Having duplicates will likely cause
/// problems.
pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut (Box<str>, Time)> + '_ {
self.0.iter_mut()
}
}
2 changes: 1 addition & 1 deletion src/run/editor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ impl Editor {
}
}

for (comparison, first_time) in first.comparisons_mut() {
for (comparison, first_time) in first.comparisons_mut().iter_mut() {
// Fix the comparison times based on the new positions of the two
// segments
let previous_time = previous
Expand Down
2 changes: 2 additions & 0 deletions src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//! ```

mod attempt;
mod comparisons;
pub mod editor;
#[cfg(feature = "std")]
pub mod parser;
Expand All @@ -29,6 +30,7 @@ mod segment_history;
mod tests;

pub use attempt::Attempt;
pub use comparisons::Comparisons;
pub use editor::{Editor, RenameError};
pub use run_metadata::{CustomVariable, RunMetadata};
pub use segment::Segment;
Expand Down
21 changes: 7 additions & 14 deletions src/run/segment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::Comparisons;
use crate::comparison::personal_best;
use crate::platform::prelude::*;
use crate::{settings::Image, SegmentHistory, Time, TimeSpan, TimingMethod};
Expand All @@ -24,7 +25,7 @@ pub struct Segment {
best_segment_time: Time,
split_time: Time,
segment_history: SegmentHistory,
comparisons: HashMap<String, Time>,
comparisons: Comparisons,
}

impl Segment {
Expand Down Expand Up @@ -70,28 +71,23 @@ impl Segment {
/// Grants mutable access to the comparison times stored in the Segment.
/// This includes both the custom comparisons and the generated ones.
#[inline]
pub fn comparisons_mut(&mut self) -> &mut HashMap<String, Time> {
pub fn comparisons_mut(&mut self) -> &mut Comparisons {
&mut self.comparisons
}

/// Grants mutable access to the specified comparison's time. If there's
/// none for this comparison, a new one is inserted with an empty time.
#[inline]
pub fn comparison_mut(&mut self, comparison: &str) -> &mut Time {
self.comparisons
.entry(comparison.into())
.or_insert_with(Time::default)
self.comparisons.get_or_insert_default(comparison)
}

/// Accesses the specified comparison's time. If there's none for this
/// comparison, an empty time is being returned (but not stored in the
/// segment).
#[inline]
pub fn comparison(&self, comparison: &str) -> Time {
self.comparisons
.get(comparison)
.cloned()
.unwrap_or_default()
self.comparisons.get(comparison).unwrap_or_default()
}

/// Accesses the given timing method of the specified comparison. If either
Expand All @@ -112,23 +108,20 @@ impl Segment {
pub fn personal_best_split_time(&self) -> Time {
self.comparisons
.get(personal_best::NAME)
.cloned()
.unwrap_or_else(Time::default)
}

/// Grants mutable access to the split time of the Personal Best for this
/// segment. If it doesn't exist an empty time is inserted.
#[inline]
pub fn personal_best_split_time_mut(&mut self) -> &mut Time {
self.comparisons
.entry(personal_best::NAME.to_string())
.or_insert_with(Time::default)
self.comparisons.get_or_insert_default(personal_best::NAME)
}

/// Sets the split time of the Personal Best to the time provided.
#[inline]
pub fn set_personal_best_split_time(&mut self, time: Time) {
self.comparisons.insert(personal_best::NAME.into(), time);
self.comparisons.set(personal_best::NAME, time);
}

/// Accesses the Best Segment Time.
Expand Down
2 changes: 1 addition & 1 deletion src/settings/gradient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};

/// Describes a Gradient for coloring a region with more than just a single
/// color.
#[derive(Debug, Copy, Clone, Serialize, Deserialize)]
#[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
pub enum Gradient {
/// Don't use any color, keep it transparent.
Transparent,
Expand Down

0 comments on commit e1dbd59

Please sign in to comment.