diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 62ab0732..737925b4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,10 +15,10 @@ jobs: steps: - name: Checkout project uses: actions/checkout@v4 - + - name: Install stable toolchain uses: dtolnay/rust-toolchain@stable - + - name: Cache dependencies uses: Swatinem/rust-cache@v2 @@ -35,7 +35,7 @@ jobs: steps: - name: Checkout project uses: actions/checkout@v4 - + - name: Install stable toolchain uses: dtolnay/rust-toolchain@stable @@ -58,10 +58,10 @@ jobs: steps: - name: Checkout project uses: actions/checkout@v4 - + - name: Install stable toolchain uses: dtolnay/rust-toolchain@stable - + - name: Cache dependencies uses: Swatinem/rust-cache@v2 @@ -75,28 +75,3 @@ jobs: --filter-expr 'test(util::sync::tests::share_gradual_taiko)' --filter-expr 'test(taiko::difficulty::gradual::tests::next_and_nth)' --no-fail-fast --failure-output=immediate-final - - non_compact: - name: Test with raw_strains feature - runs-on: ubuntu-latest - - steps: - - name: Checkout project - uses: actions/checkout@v4 - - - name: Install stable toolchain - uses: dtolnay/rust-toolchain@stable - - - name: Cache dependencies - uses: Swatinem/rust-cache@v2 - - - name: Install nextest - uses: taiki-e/install-action@nextest - - - name: Run integration tests - run: > - cargo nextest run - --no-default-features - --features raw_strains - --test '*' - --no-fail-fast --failure-output=immediate-final diff --git a/Cargo.toml b/Cargo.toml index 81aa1357..6d5f0a1b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,6 @@ keywords = ["osu", "pp", "stars", "performance", "difficulty"] [features] default = [] -raw_strains = [] sync = [] tracing = ["rosu-map/tracing"] diff --git a/README.md b/README.md index 87ceb17f..6bf5d3a5 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,6 @@ Calculating performances: Median: 40.57µs | Mean: 43.41µs | Flag | Description | Dependencies | ------------- | ------------------- | ------------ | `default` | No features enabled | -| `raw_strains` | With this feature, internal strain values will be stored in a plain `Vec`. This introduces an out-of-memory risk on maliciously long maps (see [/b/3739922](https://osu.ppy.sh/b/3739922)), but comes with a ~5% gain in performance. | | `sync` | Some gradual calculation types can only be shared across threads if this feature is enabled. This feature adds a small performance penalty. | | `tracing` | Any error encountered during beatmap decoding will be logged through `tracing::error`. If this feature is **not** enabled, errors will be ignored. | [`tracing`] diff --git a/src/any/difficulty/skills.rs b/src/any/difficulty/skills.rs index 932c4d2a..3762401a 100644 --- a/src/any/difficulty/skills.rs +++ b/src/any/difficulty/skills.rs @@ -1,4 +1,8 @@ -use crate::util::{float_ext::FloatExt, hint::unlikely, strains_vec::StrainsVec}; +use crate::util::{ + float_ext::FloatExt, + hint::unlikely, + traits::{IEnumerable, IOrderedEnumerable}, +}; pub trait StrainSkill: Sized { type DifficultyObject<'a>; @@ -26,18 +30,15 @@ pub trait StrainSkill: Sized { objects: &Self::DifficultyObjects<'a>, ); - fn into_current_strain_peaks(self) -> StrainsVec; + fn into_current_strain_peaks(self) -> Vec; - fn get_current_strain_peaks( - mut strain_peaks: StrainsVec, - current_section_peak: f64, - ) -> StrainsVec { + fn get_current_strain_peaks(mut strain_peaks: Vec, current_section_peak: f64) -> Vec { strain_peaks.push(current_section_peak); strain_peaks } - fn difficulty_value(current_strain_peaks: StrainsVec) -> f64; + fn difficulty_value(current_strain_peaks: Vec) -> f64; fn into_difficulty_value(self) -> f64; @@ -80,21 +81,17 @@ pub fn count_top_weighted_strains(object_strains: &[f64], difficulty_value: f64) .sum() } -pub fn difficulty_value(current_strain_peaks: StrainsVec, decay_weight: f64) -> f64 { +pub fn difficulty_value(current_strain_peaks: Vec, decay_weight: f64) -> f64 { let mut difficulty = 0.0; let mut weight = 1.0; // * Sections with 0 strain are excluded to avoid worst-case time complexity of the following sort (e.g. /b/2351871). // * These sections will not contribute to the difficulty. - let mut peaks = current_strain_peaks; - peaks.retain_non_zero_and_sort(); - - // SAFETY: we just removed all zeros - let peaks = unsafe { peaks.transmute_into_vec() }; + let peaks = current_strain_peaks.cs_where(|&p| p > 0.0); // * Difficulty is the weighted sum of the highest strains from every section. // * We're sorting from highest to lowest strain. - for strain in peaks { + for strain in peaks.cs_order_descending() { difficulty += strain * weight; weight *= decay_weight; } diff --git a/src/catch/strains.rs b/src/catch/strains.rs index 53573061..93b0cb83 100644 --- a/src/catch/strains.rs +++ b/src/catch/strains.rs @@ -26,6 +26,6 @@ pub fn strains(difficulty: &Difficulty, map: &Beatmap) -> Result Result [OsuDifficultyObject<'a>][OsuDifficultyObject<'a>] { include_sliders: bool, current_strain: f64 = 0.0, - slider_strains: Vec = Vec::with_capacity(64), // TODO: use `StrainsVec`? + slider_strains: Vec = Vec::with_capacity(64), } } @@ -75,7 +75,7 @@ impl Aim { // From `OsuStrainSkill`; native rather than trait function so that it has // priority over `StrainSkill::difficulty_value` - fn difficulty_value(current_strain_peaks: StrainsVec) -> f64 { + fn difficulty_value(current_strain_peaks: Vec) -> f64 { super::strain::difficulty_value( current_strain_peaks, Self::REDUCED_SECTION_COUNT, diff --git a/src/osu/difficulty/skills/flashlight.rs b/src/osu/difficulty/skills/flashlight.rs index 86282277..9ca244e0 100644 --- a/src/osu/difficulty/skills/flashlight.rs +++ b/src/osu/difficulty/skills/flashlight.rs @@ -5,7 +5,7 @@ use crate::{ skills::strain_decay, }, osu::difficulty::{evaluators::FlashlightEvaluator, object::OsuDifficultyObject}, - util::strains_vec::StrainsVec, + util::traits::IEnumerable, }; define_skill! { @@ -61,8 +61,8 @@ impl Flashlight { clippy::needless_pass_by_value, reason = "function definition needs to stay in-sync with `StrainSkill::difficulty_value`" )] - fn difficulty_value(current_strain_peaks: StrainsVec) -> f64 { - current_strain_peaks.sum() + fn difficulty_value(current_strain_peaks: Vec) -> f64 { + current_strain_peaks.cs_sum() } pub fn difficulty_to_performance(difficulty: f64) -> f64 { diff --git a/src/osu/difficulty/skills/speed.rs b/src/osu/difficulty/skills/speed.rs index 5312b3d1..43655222 100644 --- a/src/osu/difficulty/skills/speed.rs +++ b/src/osu/difficulty/skills/speed.rs @@ -7,7 +7,6 @@ use crate::{ evaluators::{RhythmEvaluator, SpeedEvaluator}, object::OsuDifficultyObject, }, - util::strains_vec::StrainsVec, }; use super::strain::OsuStrainSkill; @@ -86,7 +85,7 @@ impl Speed { // From `OsuStrainSkill`; native rather than trait function so that it has // priority over `StrainSkill::difficulty_value` - fn difficulty_value(current_strain_peaks: StrainsVec) -> f64 { + fn difficulty_value(current_strain_peaks: Vec) -> f64 { super::strain::difficulty_value( current_strain_peaks, Self::REDUCED_SECTION_COUNT, diff --git a/src/osu/difficulty/skills/strain.rs b/src/osu/difficulty/skills/strain.rs index c2256e9b..265944d5 100644 --- a/src/osu/difficulty/skills/strain.rs +++ b/src/osu/difficulty/skills/strain.rs @@ -1,4 +1,8 @@ -use crate::util::{difficulty::logistic, float_ext::FloatExt, strains_vec::StrainsVec}; +use crate::util::{ + difficulty::logistic, + float_ext::FloatExt, + traits::{IEnumerable, IOrderedEnumerable}, +}; pub trait OsuStrainSkill { const REDUCED_SECTION_COUNT: usize = 10; @@ -10,7 +14,7 @@ pub trait OsuStrainSkill { } pub fn difficulty_value( - current_strain_peaks: StrainsVec, + current_strain_peaks: Vec, reduced_section_count: usize, reduced_strain_baseline: f64, decay_weight: f64, @@ -18,31 +22,19 @@ pub fn difficulty_value( let mut difficulty = 0.0; let mut weight = 1.0; - let mut peaks = current_strain_peaks; + // * Sections with 0 strain are excluded to avoid worst-case time complexity of the following sort (e.g. /b/2351871). + // * These sections will not contribute to the difficulty. + let peaks = current_strain_peaks.cs_where(|&p| p > 0.0); - // Note that we remove all initial zeros here. - let peaks_iter = peaks.sorted_non_zero_iter_mut().take(reduced_section_count); + let mut strains = peaks.cs_order_descending(); - for (i, strain) in peaks_iter.enumerate() { - // Note that unless `reduced_strain_baseline == 0.0`, `strain` can - // never be `0.0`. + for (i, strain) in strains.iter_mut().take(reduced_section_count).enumerate() { let clamped = f64::from((i as f32 / reduced_section_count as f32).clamp(0.0, 1.0)); let scale = f64::log10(lerp(1.0, 10.0, clamped)); *strain *= lerp(reduced_strain_baseline, 1.0, scale); } - peaks.sort_desc(); - - // Sanity assert; will most definitely never panic - debug_assert!(reduced_strain_baseline != 0.0); - - // SAFETY: As noted, zeros were removed from all initial strains and no - // strain was mutated to a zero afterwards. - let peaks = unsafe { peaks.transmute_into_vec() }; - - // Using `Vec` is much faster for iteration than `StrainsVec` - - for strain in peaks { + for strain in strains.cs_order_descending() { difficulty += strain * weight; weight *= decay_weight; } diff --git a/src/osu/strains.rs b/src/osu/strains.rs index c185e6b5..8afbdd92 100644 --- a/src/osu/strains.rs +++ b/src/osu/strains.rs @@ -40,9 +40,9 @@ pub fn strains(difficulty: &Difficulty, map: &Beatmap) -> Result Result = Vec::with_capacity(256), // <- strain_skill_object_strains Vec = Vec::with_capacity(256), // <- } $( $rest )* @@ -247,7 +245,6 @@ macro_rules! define_skill { object::{IDifficultyObject, IDifficultyObjects, HasStartTime}, skills::{StrainSkill, StrainDecaySkill}, }, - util::strains_vec::StrainsVec, }; define_skill!( @impl $trait $name $objects[$object] ); @@ -316,14 +313,14 @@ macro_rules! define_skill { = self.calculate_initial_strain(time, curr, objects); } - fn into_current_strain_peaks(self) -> StrainsVec { + fn into_current_strain_peaks(self) -> Vec { Self::get_current_strain_peaks( self.strain_skill_strain_peaks, self.strain_skill_current_section_peak, ) } - fn difficulty_value(current_strain_peaks: StrainsVec) -> f64 { + fn difficulty_value(current_strain_peaks: Vec) -> f64 { crate::any::difficulty::skills::difficulty_value( current_strain_peaks, Self::DECAY_WEIGHT, diff --git a/src/util/mod.rs b/src/util/mod.rs index b4f7a216..33c6ebb0 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -6,8 +6,8 @@ pub mod map_or_attrs; pub mod random; pub mod ruleset_ext; pub mod sort; -pub mod strains_vec; pub mod sync; +pub mod traits; #[macro_use] mod macros; diff --git a/src/util/strains_vec.rs b/src/util/strains_vec.rs deleted file mode 100644 index 029a6180..00000000 --- a/src/util/strains_vec.rs +++ /dev/null @@ -1,444 +0,0 @@ -// TODO: remove; suspicion checks should now be used to prevent calculation on -// malicious maps - -pub use inner::*; - -#[cfg(not(feature = "raw_strains"))] -mod inner { - use std::{ - iter::{self, Copied}, - mem, - slice::{self, Iter}, - }; - - use crate::util::hint::{likely, unlikely}; - - use self::entry::StrainsEntry; - - /// A specialized `Vec` where all entries must be non-negative. - /// - /// It is compact in the sense that zeros are not stored directly but instead - /// as amount of times they appear consecutively. - /// - /// For cases with few consecutive zeros, this type generally reduces - /// performance slightly. However, for edge cases like `/b/3739922` the length - /// of the list is massively reduced, preventing out-of-memory issues. - #[derive(Clone)] - pub struct StrainsVec { - inner: Vec, - len: usize, - #[cfg(debug_assertions)] - // Ensures that methods are used correctly - has_zero: bool, - } - - impl StrainsVec { - /// Constructs a new, empty [`StrainsVec`] with at least the specified - /// capacity. - #[inline] - pub fn with_capacity(capacity: usize) -> Self { - Self { - inner: Vec::with_capacity(capacity), - len: 0, - #[cfg(debug_assertions)] - has_zero: false, - } - } - - /// Returns the number of elements. - #[inline] - pub const fn len(&self) -> usize { - self.len - } - - /// Appends an element to the back. - #[inline] - pub fn push(&mut self, value: f64) { - if likely(value.to_bits() > 0 && value.is_sign_positive()) { - // SAFETY: we just checked whether it's positive - self.inner.push(unsafe { StrainsEntry::new_value(value) }); - } else if let Some(last) = self.inner.last_mut().filter(|e| e.is_zero()) { - last.incr_zero_count(); - } else { - self.inner.push(StrainsEntry::new_zero()); - - #[cfg(debug_assertions)] - { - self.has_zero = true; - } - } - - self.len += 1; - } - - /// Sorts the entries in descending order. - #[inline] - pub fn sort_desc(&mut self) { - #[cfg(debug_assertions)] - debug_assert!(!self.has_zero); - - self.inner.sort_by(|a, b| b.value().total_cmp(&a.value())); - } - - /// Removes all zero entries - #[inline] - pub fn retain_non_zero(&mut self) { - self.inner.retain(|e| likely(e.is_value())); - - #[cfg(debug_assertions)] - { - self.has_zero = false; - } - } - - /// Removes all zeros and sorts the remaining entries in descending order. - #[inline] - pub fn retain_non_zero_and_sort(&mut self) { - self.retain_non_zero(); - self.sort_desc(); - } - - /// Removes all zeros, sorts the remaining entries in descending order, and - /// returns an iterator over mutable references to the values. - #[inline] - pub fn sorted_non_zero_iter_mut(&mut self) -> impl ExactSizeIterator { - self.retain_non_zero_and_sort(); - - self.inner.iter_mut().map(StrainsEntry::as_value_mut) - } - - /// Sum up all values. - #[inline] - pub fn sum(&self) -> f64 { - self.inner - .iter() - .copied() - .filter_map(StrainsEntry::try_as_value) - .sum() - } - - /// Returns an iterator over the [`StrainsVec`]. - #[inline] - pub fn iter(&self) -> StrainsIter<'_> { - StrainsIter::new(self) - } - - /// Converts this [`StrainsVec`] into `Vec`. - /// - /// # Safety - /// - /// `self` may not include *any* zeros. - pub unsafe fn transmute_into_vec(self) -> Vec { - // SAFETY: `StrainsEntry` has the same properties as `f64` - unsafe { mem::transmute::, Vec>(self.inner) } - } - - /// Allocates a new `Vec` to store all values, including zeros. - pub fn into_vec(self) -> Vec { - /// Copies the first `count` items of `slice` into `dst`. - fn copy_slice(slice: &[StrainsEntry], count: usize, dst: &mut Vec) { - if unlikely(count == 0) { - return; - } - - let ptr = slice.as_ptr().cast(); - - // SAFETY: `StrainsEntry` has the same properties as `f64` - let slice = unsafe { slice::from_raw_parts(ptr, count) }; - dst.extend_from_slice(slice); - } - - /// Drives the iterator until it finds a zero count. It then copies - /// entries up to that and returns the zero count. - #[inline] - fn copy_non_zero( - iter: &mut Iter<'_, StrainsEntry>, - dst: &mut Vec, - ) -> Option { - let mut count = 0; - let slice = iter.as_slice(); - - for entry in iter { - if unlikely(entry.is_zero()) { - copy_slice(slice, count, dst); - - return Some(entry.zero_count() as usize); - } - - count += 1; - } - - copy_slice(slice, count, dst); - - None - } - - let mut vec = Vec::with_capacity(self.len); - let mut iter = self.inner.iter(); - - while let Some(zero_count) = copy_non_zero(&mut iter, &mut vec) { - vec.extend(iter::repeat_n(0.0, zero_count)); - } - - vec - } - } - - pub struct StrainsIter<'a> { - inner: Copied>, - curr: Option, - len: usize, - } - - impl<'a> StrainsIter<'a> { - pub fn new(vec: &'a StrainsVec) -> Self { - let mut inner = vec.inner.iter().copied(); - let curr = inner.next(); - - Self { - inner, - curr, - len: vec.len, - } - } - } - - impl Iterator for StrainsIter<'_> { - type Item = f64; - - fn next(&mut self) -> Option { - loop { - let curr = self.curr.as_mut()?; - - if likely(curr.is_value()) { - let value = curr.value(); - self.curr = self.inner.next(); - self.len -= 1; - - return Some(value); - } else if curr.zero_count() > 0 { - curr.decr_zero_count(); - self.len -= 1; - - return Some(0.0); - } - - self.curr = self.inner.next(); - } - } - - fn size_hint(&self) -> (usize, Option) { - let len = self.len(); - - (len, Some(len)) - } - } - - impl ExactSizeIterator for StrainsIter<'_> { - fn len(&self) -> usize { - self.len - } - } - - /// Private module to hide internal fields. - mod entry { - use super::likely; - - /// Either a positive `f64` or an amount of consecutive `0.0`. - /// - /// If the first bit is not set, i.e. the sign bit of a `f64` indicates - /// that it's positive, the union represents that `f64`. Otherwise, the - /// first bit is ignored and the union represents a `u64`. - #[derive(Copy, Clone)] - pub union StrainsEntry { - value: f64, - zero_count: u64, - } - - impl StrainsEntry { - const ZERO_COUNT_MASK: u64 = u64::MAX >> 1; - - /// # Safety - /// - /// `value` must be positive, i.e. neither negative nor zero. - #[inline] - pub const unsafe fn new_value(value: f64) -> Self { - Self { value } - } - - #[inline] - pub const fn new_zero() -> Self { - Self { - zero_count: !Self::ZERO_COUNT_MASK + 1, - } - } - - #[inline] - pub const fn is_zero(self) -> bool { - unsafe { self.value.is_sign_negative() } - } - - #[inline] - pub const fn is_value(self) -> bool { - !self.is_zero() - } - - #[inline] - pub const fn value(self) -> f64 { - unsafe { self.value } - } - - #[inline] - pub const fn try_as_value(self) -> Option { - if likely(self.is_value()) { - Some(self.value()) - } else { - None - } - } - - #[inline] - pub const fn as_value_mut(&mut self) -> &mut f64 { - unsafe { &mut self.value } - } - - #[inline] - pub const fn zero_count(self) -> u64 { - unsafe { self.zero_count & Self::ZERO_COUNT_MASK } - } - - #[inline] - pub const fn incr_zero_count(&mut self) { - unsafe { - self.zero_count += 1; - } - } - - #[inline] - pub const fn decr_zero_count(&mut self) { - unsafe { - self.zero_count -= 1; - } - } - } - } - - #[cfg(test)] - mod tests { - use proptest::prelude::*; - - use crate::util::float_ext::FloatExt; - - use super::*; - - proptest! { - #[test] - fn expected(values in prop::collection::vec(prop::option::of(0.0..1_000.0), 0..1_000)) { - let mut vec = StrainsVec::with_capacity(values.len()); - let mut raw = Vec::with_capacity(values.len()); - - let mut additional_zeros = 0; - let mut prev_zero = false; - let mut sum = 0.0; - - for opt in values.iter().copied() { - if let Some(value) = opt.filter(|&value| value != 0.0) { - let value = f64::abs(value); - - vec.push(value); - raw.push(value); - prev_zero = false; - sum += value; - } else { - vec.push(0.0); - raw.push(0.0); - - if prev_zero { - additional_zeros += 1; - } - - prev_zero = true; - } - } - - assert_eq!(vec.len(), raw.len()); - assert_eq!(vec.inner.len(), raw.len() - additional_zeros); - assert!(vec.sum().eq(sum)); - assert!(vec.iter().eq(raw.iter().copied())); - assert_eq!(vec.clone().into_vec(), raw); - - vec.retain_non_zero_and_sort(); - raw.retain(|&n| n > 0.0); - raw.sort_by(|a, b| b.total_cmp(a)); - - assert_eq!(unsafe { vec.transmute_into_vec() }, raw); - } - } - } -} - -#[cfg(feature = "raw_strains")] -mod inner { - use std::{ - iter::Copied, - slice::{Iter, IterMut}, - }; - - /// Plain wrapper around `Vec` because the `raw_strains` feature - /// is disabled. - #[derive(Clone)] - pub struct StrainsVec { - inner: Vec, - } - - impl StrainsVec { - pub fn with_capacity(capacity: usize) -> Self { - Self { - inner: Vec::with_capacity(capacity), - } - } - - pub fn len(&self) -> usize { - self.inner.len() - } - - pub fn push(&mut self, value: f64) { - self.inner.push(value); - } - - pub fn sort_desc(&mut self) { - self.inner.sort_by(|a, b| b.total_cmp(a)); - } - - pub fn retain_non_zero(&mut self) { - self.inner.retain(|&a| a > 0.0); - } - - pub fn retain_non_zero_and_sort(&mut self) { - self.retain_non_zero(); - self.sort_desc(); - } - - pub fn sorted_non_zero_iter_mut(&mut self) -> IterMut<'_, f64> { - self.retain_non_zero_and_sort(); - - self.inner.iter_mut() - } - - pub fn sum(&self) -> f64 { - self.inner.iter().copied().sum() - } - - pub fn iter(&self) -> Copied> { - self.inner.iter().copied() - } - - pub unsafe fn transmute_into_vec(self) -> Vec { - self.inner - } - - pub fn into_vec(self) -> Vec { - self.inner - } - } -} diff --git a/src/util/traits.rs b/src/util/traits.rs new file mode 100644 index 00000000..1205addc --- /dev/null +++ b/src/util/traits.rs @@ -0,0 +1,51 @@ +use std::iter::Sum; + +/// Mimics the C# `IEnumerable` interface. +pub trait IEnumerable: Sized { + fn cs_where bool>(self, f: F) -> Self; + + fn cs_sum(&self) -> S + where + S: for<'a> Sum<&'a T>; +} + +/// Mimics the C# `IOrderedEnumerable` interface. +pub trait IOrderedEnumerable: IEnumerable { + fn cs_order_descending(self) -> Self; +} + +impl IEnumerable for Vec { + /// Filters a sequence of values based on a predicate. + /// + /// + fn cs_where bool>(mut self, f: F) -> Self { + self.retain(f); + + self + } + + /// Computes the sum of a sequence of numeric values. + /// + /// + fn cs_sum(&self) -> S + where + S: for<'a> Sum<&'a T>, + { + self.iter().sum() + } +} + +impl IOrderedEnumerable for Vec { + /// Sorts the elements of a sequence in descending order. + /// + /// This method performs a stable sort; that is, if the keys of two elements + /// are equal, the order of the elements is preserved. In contrast, an unstable sort does not + /// preserve the order of elements that have the same key. + /// + /// + fn cs_order_descending(mut self) -> Self { + self.sort_by(|a, b| b.total_cmp(a)); + + self + } +}