Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up the comparison time lookups #338

Merged
merged 2 commits into from
Jun 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/rendering/glyph_cache.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use super::mesh::{fill_builder, Mesh};
use super::Backend;
use hashbrown::HashMap;
use lyon::{
path::{self, math::point, Path},
tessellation::{FillOptions, FillTessellator},
};
use rusttype::{Font, GlyphId, OutlineBuilder, Scale};
use std::collections::HashMap;

struct PathBuilder(path::Builder);

Expand Down
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 over the few comparisons we have and
// compare them directly. Most will be rejected right away since 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 addition, Personal Best will be the first
// comparison in the list most of the time, and that's the one we want to look
// up most often 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
22 changes: 7 additions & 15 deletions src/run/segment.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::Comparisons;
use crate::comparison::personal_best;
use crate::platform::prelude::*;
use crate::{settings::Image, SegmentHistory, Time, TimeSpan, TimingMethod};
use hashbrown::HashMap;

/// A Segment describes a point in a speedrun that is suitable for storing a
/// split time. This stores the name of that segment, an icon, the split times
Expand All @@ -24,7 +24,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 +70,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 +107,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