From 710f53c981201efb3bacaeac21ccfbd96aa200c6 Mon Sep 17 00:00:00 2001
From: Christopher Serr <christopher.serr@gmail.com>
Date: Tue, 9 Jun 2020 16:16:04 +0200
Subject: [PATCH 1/2] Speed up the comparison time lookups

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. 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.
---
 src/rendering/glyph_cache.rs |  2 +-
 src/run/comparisons.rs       | 83 ++++++++++++++++++++++++++++++++++++
 src/run/editor/mod.rs        |  2 +-
 src/run/mod.rs               |  2 +
 src/run/segment.rs           | 22 +++-------
 src/settings/gradient.rs     |  2 +-
 6 files changed, 95 insertions(+), 18 deletions(-)
 create mode 100644 src/run/comparisons.rs

diff --git a/src/rendering/glyph_cache.rs b/src/rendering/glyph_cache.rs
index 8a809c3d..90c4f748 100644
--- a/src/rendering/glyph_cache.rs
+++ b/src/rendering/glyph_cache.rs
@@ -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);
 
diff --git a/src/run/comparisons.rs b/src/run/comparisons.rs
new file mode 100644
index 00000000..fb9bfb86
--- /dev/null
+++ b/src/run/comparisons.rs
@@ -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()
+    }
+}
diff --git a/src/run/editor/mod.rs b/src/run/editor/mod.rs
index 3de87782..945e5298 100644
--- a/src/run/editor/mod.rs
+++ b/src/run/editor/mod.rs
@@ -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
diff --git a/src/run/mod.rs b/src/run/mod.rs
index 8694b96a..d17cfcb5 100644
--- a/src/run/mod.rs
+++ b/src/run/mod.rs
@@ -16,6 +16,7 @@
 //! ```
 
 mod attempt;
+mod comparisons;
 pub mod editor;
 #[cfg(feature = "std")]
 pub mod parser;
@@ -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;
diff --git a/src/run/segment.rs b/src/run/segment.rs
index 97a46acd..6cb94d6f 100644
--- a/src/run/segment.rs
+++ b/src/run/segment.rs
@@ -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
@@ -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 {
@@ -70,7 +70,7 @@ 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
     }
 
@@ -78,9 +78,7 @@ impl Segment {
     /// 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
@@ -88,10 +86,7 @@ impl Segment {
     /// 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
@@ -112,7 +107,6 @@ impl Segment {
     pub fn personal_best_split_time(&self) -> Time {
         self.comparisons
             .get(personal_best::NAME)
-            .cloned()
             .unwrap_or_else(Time::default)
     }
 
@@ -120,15 +114,13 @@ impl Segment {
     /// 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.
diff --git a/src/settings/gradient.rs b/src/settings/gradient.rs
index 9509cc1f..bc16921e 100644
--- a/src/settings/gradient.rs
+++ b/src/settings/gradient.rs
@@ -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,

From 598941e4bc18cf45190148ee5b77dd1b586edc51 Mon Sep 17 00:00:00 2001
From: Christopher Serr <christopher.serr@gmail.com>
Date: Thu, 11 Jun 2020 14:05:28 +0200
Subject: [PATCH 2/2] Update src/run/comparisons.rs

Co-authored-by: wooferzfg <spapushin@gmail.com>
---
 src/run/comparisons.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/run/comparisons.rs b/src/run/comparisons.rs
index fb9bfb86..0b9b5053 100644
--- a/src/run/comparisons.rs
+++ b/src/run/comparisons.rs
@@ -5,19 +5,19 @@ use crate::Time;
 // 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.
+// 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
+// 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
+// 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.