Skip to content

Commit

Permalink
#79 add UniCase for faster case-insensitive comparisons, faster `fi…
Browse files Browse the repository at this point in the history
…nd` fns
  • Loading branch information
fsktom committed Aug 22, 2024
1 parent a4f0bf2 commit b764ae0
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 84 deletions.
16 changes: 16 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ chrono = "0.4"
itertools = "0.13"
tracing = "0.1"
thiserror = "1.0"
unicase = "2.7"

[dev-dependencies]
criterion = "0.5"
Expand Down
80 changes: 33 additions & 47 deletions benches/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ fn parse(c: &mut Criterion) {
})
});

// let a = SongEntries::new(&paths)
// .unwrap()
// .sum_different_capitalization()
// .filter(30, TimeDelta::seconds(10));
// let b = SongEntries::new(&paths)
// .unwrap()
// .new_sum_different_capitalization()
// .filter(30, TimeDelta::seconds(10));
// assert!(a.iter().eq(b.iter()));

c.bench_function("parse, sum and filter", |c| {
c.iter(|| {
black_box(
Expand All @@ -101,6 +111,17 @@ fn parse(c: &mut Criterion) {
);
})
});

// c.bench_function("new parse, sum and filter", |c| {
// c.iter(|| {
// black_box(
// SongEntries::new(&paths)
// .unwrap()
// .new_sum_different_capitalization()
// .filter(30, TimeDelta::seconds(10)),
// );
// })
// });
}

// not related to this at all but just curious xd
Expand Down Expand Up @@ -213,52 +234,14 @@ fn capitalization(c: &mut Criterion) {
#[allow(dead_code)]
fn find(c: &mut Criterion) {
let entries = black_box(SongEntries::new(&paths(0, 9)).unwrap());
let usr_song_one = Song::new("MUKANJYO", "MUKANJYO", "Survive Said The Prophet");
let usr_song_two = Song::new("MUKANJYO", "Mukanjyo", "Survive Said The Prophet");

let one = entries
.iter()
.find(|entry| usr_song_one.is_entry_lowercase(entry))
.map(Song::from)
.unwrap();
let two = entries
.iter()
.find(|entry| usr_song_two.is_entry_lowercase(entry))
.map(Song::from)
.unwrap();
dbg!(one, two);

let (song_name, artist_name) = (
usr_song_one.name.to_lowercase(),
usr_song_one.album.artist.name.to_lowercase(),
);

let onetwo = entries
.iter()
.filter(|entry| {
entry.track.to_lowercase() == song_name && entry.artist.to_lowercase() == artist_name
})
.unique()
.map(Song::from)
.collect_vec();
dbg!(onetwo);

c.bench_function("find v1", |c| {
c.iter(|| {
entries
.iter()
.find(|entry| usr_song_one.is_entry_lowercase(entry))
.map(Song::from)
})
});
c.bench_function("find v2", |c| {
c.iter(|| {
entries
.iter()
.filter(|entry| usr_song_one.is_entry_lowercase(entry))
.map(Song::from)
.unique()
.collect_vec()
black_box(entries.find().song_from_album(
"MUKANJYO",
"MUKANJYO",
"Survive Said The Prophet",
));
})
});

Expand All @@ -267,27 +250,30 @@ fn find(c: &mut Criterion) {
c.iter(|| {
entries
.iter()
.find(|entry| art.is_entry_lowercase(entry))
.find(|entry| art.is_entry_ignore_case(entry))
.map(Artist::from)
})
});
c.bench_function("find artist v2", |c| {
c.iter(|| {
entries
.iter()
.filter(|entry| art.is_entry_lowercase(entry))
.filter(|entry| art.is_entry_ignore_case(entry))
.map(Artist::from)
.unique()
.collect_vec()
})
});
c.bench_function("find song", |c| {
c.iter(|| black_box(entries.find().song("mukanjyo", "survive said THE PROPHET")))
});
}

// criterion_group!(benches, lol);
// criterion_group!(benches, kekw);
criterion_group!(benches, parse);
// criterion_group!(benches, parse);
// criterion_group!(benches, unique_sum);
// criterion_group!(benches, gather);
criterion_group!(benches, gather);
// criterion_group!(benches, capitalization);
// criterion_group!(benches, find);
criterion_main!(benches);
1 change: 1 addition & 0 deletions endsong_ui/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions endsong_ui/src/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub fn absolute_ignore_album(entries: &SongEntries, song: &Song) -> TraceType {

for entry in entries
.iter()
.filter(|entry| song.is_entry_lowercase_ignore_album(entry))
.filter(|entry| song.is_entry_ignore_album_and_case(entry))
{
song_plays += 1;
times.push(format_date(&entry.timestamp));
Expand Down Expand Up @@ -224,7 +224,7 @@ pub mod relative {
for entry in entries.iter() {
all_plays += 1.0;

if song.is_entry_lowercase_ignore_album(entry) {
if song.is_entry_ignore_album_and_case(entry) {
aspect_found = true;
aspect_plays += 1.0;
}
Expand Down Expand Up @@ -263,7 +263,7 @@ pub mod relative {
for entry in entries.iter().filter(|entry| artist.is_entry(entry)) {
artist_plays += 1.0;

if song.is_entry_lowercase_ignore_album(entry) {
if song.is_entry_ignore_album_and_case(entry) {
aspect_found = true;
aspect_plays += 1.0;
}
Expand Down
61 changes: 41 additions & 20 deletions src/aspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ use std::cmp::Ordering;
use std::fmt::Display;
use std::rc::Rc;

use unicase::UniCase;

use crate::entry::SongEntry;

/// Used for functions that accept either
Expand All @@ -57,10 +59,10 @@ pub trait Music: Display + Clone + Eq + Ord {
/// Checks if a [`SongEntry`] is a [`Music`]
fn is_entry(&self, entry: &SongEntry) -> bool;

/// Checks if a [`SongEntry`] is a [`Music`] but case insensitive
/// Checks if a [`SongEntry`] is a [`Music`] but case-insensitive
///
/// Performs `.to_lowercase()` on both `entry` and on [`self`].
fn is_entry_lowercase(&self, entry: &SongEntry) -> bool;
/// Uses [`UniCase`] for fast case-insensitive comparison `entry` between [`self`].
fn is_entry_ignore_case(&self, entry: &SongEntry) -> bool;
}

/// Trait used to accept only [`Artist`] and [`Album`]
Expand Down Expand Up @@ -150,8 +152,8 @@ impl Music for Artist {
fn is_entry(&self, entry: &SongEntry) -> bool {
entry.artist == self.name
}
fn is_entry_lowercase(&self, entry: &SongEntry) -> bool {
entry.artist.to_lowercase() == self.name.to_lowercase()
fn is_entry_ignore_case(&self, entry: &SongEntry) -> bool {
UniCase::new(&entry.artist) == UniCase::new(&self.name)
}
}
impl HasSongs for Artist {}
Expand Down Expand Up @@ -256,11 +258,11 @@ impl AsRef<str> for Album {
}
impl Music for Album {
fn is_entry(&self, entry: &SongEntry) -> bool {
entry.artist == self.artist.name && entry.album == self.name
self.artist.is_entry(entry) && entry.album == self.name
}
fn is_entry_lowercase(&self, entry: &SongEntry) -> bool {
entry.artist.to_lowercase() == self.artist.name.to_lowercase()
&& entry.album.to_lowercase() == self.name.to_lowercase()
fn is_entry_ignore_case(&self, entry: &SongEntry) -> bool {
self.artist.is_entry_ignore_case(entry)
&& UniCase::new(&entry.album) == UniCase::new(&self.name)
}
}
impl HasSongs for Album {}
Expand Down Expand Up @@ -292,11 +294,10 @@ impl Song {
}

/// Checks if a [`SongEntry`] is this song, but only regarding artist and track name,
/// ignoring album name
/// ignoring album name and capitalization for track name
#[must_use]
pub fn is_entry_lowercase_ignore_album(&self, entry: &SongEntry) -> bool {
entry.artist.to_lowercase() == self.album.artist.name.to_lowercase()
&& entry.track.to_lowercase() == self.name.to_lowercase()
pub fn is_entry_ignore_album_and_case(&self, entry: &SongEntry) -> bool {
self.album.artist.is_entry(entry) && UniCase::new(&entry.track) == UniCase::new(&self.name)
}
}
impl Clone for Song {
Expand Down Expand Up @@ -380,14 +381,11 @@ impl AsRef<str> for Song {
}
impl Music for Song {
fn is_entry(&self, entry: &SongEntry) -> bool {
entry.artist == self.album.artist.name
&& entry.album == self.album.name
&& entry.track == self.name
self.album.is_entry(entry) && entry.track == self.name
}
fn is_entry_lowercase(&self, entry: &SongEntry) -> bool {
entry.artist.to_lowercase() == self.album.artist.name.to_lowercase()
&& entry.album.to_lowercase() == self.album.name.to_lowercase()
&& entry.track.to_lowercase() == self.name.to_lowercase()
fn is_entry_ignore_case(&self, entry: &SongEntry) -> bool {
self.album.is_entry_ignore_case(entry)
&& UniCase::new(&entry.track) == UniCase::new(&self.name)
}
}

Expand Down Expand Up @@ -547,6 +545,29 @@ mod tests {
);
}

#[test]
fn aspect_equality() {
let entry = SongEntry {
timestamp: crate::parse_date("now").unwrap(),
time_played: chrono::TimeDelta::zero(),
track: Rc::from("White Death"),
album: Rc::from("Coat of Arms"),
artist: Rc::from("Sabaton"),
};

assert!(Artist::new("Sabaton").is_entry(&entry));
assert!(!Artist::new("SABATON").is_entry(&entry));
assert!(Artist::new("SABATON").is_entry_ignore_case(&entry));

assert!(Album::new("Coat of Arms", "Sabaton").is_entry(&entry));
assert!(!Album::new("Coat Of Arms", "SABATON").is_entry(&entry));
assert!(Album::new("Coat Of Arms", "SABATON").is_entry_ignore_case(&entry));

assert!(Song::new("White Death", "Coat of Arms", "Sabaton").is_entry(&entry));
assert!(!Song::new("white death", "Coat Of Arms", "SABATON").is_entry(&entry));
assert!(Song::new("white death", "Coat Of Arms", "SABATON").is_entry_ignore_case(&entry));
}

#[test]
fn test_dates() {
// MAYBE RATHER INTEGRATION TEST THAN UNIT TEST?!
Expand Down
6 changes: 3 additions & 3 deletions src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ pub struct SongEntry {
pub album: Rc<str>,
/// name of the artist
pub artist: Rc<str>,
/// Spotify URI
pub id: String,
// /// Spotify URI
// pub id: String,
}
/// Equal if `artist`, `album` and `track` name are the same
impl PartialEq for SongEntry {
Expand Down Expand Up @@ -174,7 +174,7 @@ impl SongEntries {

// the last album in the vector is the one that will be kept
// cause it's the most recent one
// key: albym, value: newest album name
// key: album, value: newest album name
let mut album_mappings: HashMap<Album, Rc<str>> = HashMap::new();

for alb in albums {
Expand Down
12 changes: 5 additions & 7 deletions src/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn artist(entries: &[SongEntry], artist_name: &str) -> Option<Vec<Artist>> {

let artists = entries
.iter()
.filter(|entry| usr_artist.is_entry_lowercase(entry))
.filter(|entry| usr_artist.is_entry_ignore_case(entry))
.map(Artist::from)
.unique()
.collect_vec();
Expand Down Expand Up @@ -65,7 +65,7 @@ pub fn album(entries: &[SongEntry], album_name: &str, artist_name: &str) -> Opti

let albums = entries
.iter()
.filter(|entry| usr_album.is_entry_lowercase(entry))
.filter(|entry| usr_album.is_entry_ignore_case(entry))
.map(Album::from)
.unique()
.collect_vec();
Expand Down Expand Up @@ -99,7 +99,7 @@ pub fn song_from_album(

let songs = entries
.iter()
.filter(|entry| usr_song.is_entry_lowercase(entry))
.filter(|entry| usr_song.is_entry_ignore_case(entry))
.map(Song::from)
.unique()
.collect_vec();
Expand All @@ -119,13 +119,11 @@ pub fn song_from_album(
///
/// See #2 <https://github.com/fsktom/rusty-endsong-parser/issues/2>
pub fn song(entries: &[SongEntry], song_name: &str, artist_name: &str) -> Option<Vec<Song>> {
let (song_name, artist_name) = (song_name.to_lowercase(), artist_name.to_lowercase());
let song_bogus_album = Song::new(song_name, "", artist_name);

let song_versions = entries
.iter()
.filter(|entry| {
entry.track.to_lowercase() == song_name && entry.artist.to_lowercase() == artist_name
})
.filter(|entry| song_bogus_album.is_entry_ignore_album_and_case(entry))
.map(Song::from)
.unique()
.collect_vec();
Expand Down
Loading

0 comments on commit b764ae0

Please sign in to comment.