From 1c40ac76730f7868f385cc388561fc0803a4f18c Mon Sep 17 00:00:00 2001 From: Polochon-street Date: Sun, 9 Oct 2022 12:07:34 +0200 Subject: [PATCH] Fix a bug in CPU number --- CHANGELOG.md | 3 ++ Cargo.lock | 2 +- Cargo.toml | 2 +- examples/library.rs | 3 +- examples/library_extra_info.rs | 3 +- src/lib.rs | 13 ++--- src/library.rs | 88 +++++++++++++++++++++++++++++----- 7 files changed, 92 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 587a8f9..f5954f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ #Changelog +## bliss 0.6.4 +* Fix a bug in the customizable CPU number option in `library`. + ## bliss 0.6.3 * Add customizable CPU number in the `library` module. diff --git a/Cargo.lock b/Cargo.lock index a6215d7..8ad6631 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -96,7 +96,7 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bliss-audio" -version = "0.6.3" +version = "0.6.4" dependencies = [ "anyhow", "bliss-audio-aubio-rs", diff --git a/Cargo.toml b/Cargo.toml index 6591a1b..e32bd00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bliss-audio" -version = "0.6.3" +version = "0.6.4" authors = ["Polochon-street "] edition = "2018" license = "GPL-3.0-only" diff --git a/examples/library.rs b/examples/library.rs index 3398136..b178ff0 100644 --- a/examples/library.rs +++ b/examples/library.rs @@ -9,6 +9,7 @@ use clap::{App, Arg, SubCommand}; use glob::glob; use serde::{Deserialize, Serialize}; use std::fs; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; #[derive(Serialize, Deserialize, Clone, Debug)] @@ -30,7 +31,7 @@ impl Config { music_library_path: PathBuf, config_path: Option, database_path: Option, - number_cores: Option, + number_cores: Option, ) -> Result { let base_config = BaseConfig::new(config_path, database_path, number_cores)?; Ok(Self { diff --git a/examples/library_extra_info.rs b/examples/library_extra_info.rs index c024512..8bfe931 100644 --- a/examples/library_extra_info.rs +++ b/examples/library_extra_info.rs @@ -10,6 +10,7 @@ use clap::{App, Arg, SubCommand}; use glob::glob; use serde::{Deserialize, Serialize}; use std::fs; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; #[derive(Serialize, Deserialize, Clone, Debug)] @@ -31,7 +32,7 @@ impl Config { music_library_path: PathBuf, config_path: Option, database_path: Option, - number_cores: Option, + number_cores: Option, ) -> Result { let base_config = BaseConfig::new(config_path, database_path, number_cores)?; Ok(Self { diff --git a/src/lib.rs b/src/lib.rs index 19a51d9..92bce1d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,6 +83,7 @@ extern crate num_cpus; extern crate serde; use crate::cue::BlissCue; use log::info; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::mpsc; use std::thread; @@ -155,7 +156,7 @@ pub type BlissResult = Result; pub fn analyze_paths, F: IntoIterator>( paths: F, ) -> mpsc::IntoIter<(PathBuf, BlissResult)> { - let cores = num_cpus::get(); + let cores = NonZeroUsize::new(num_cpus::get()).unwrap(); analyze_paths_with_cores(paths, cores) } @@ -186,11 +187,11 @@ pub fn analyze_paths, F: IntoIterator>( /// /// # Example: /// ```no_run -/// use bliss_audio::{analyze_paths_with_cores, BlissResult}; +/// use bliss_audio::{analyze_paths, BlissResult}; /// /// fn main() -> BlissResult<()> { /// let paths = vec![String::from("/path/to/song1"), String::from("/path/to/song2")]; -/// for (path, result) in analyze_paths_with_cores(&paths, 2) { +/// for (path, result) in analyze_paths(&paths) { /// match result { /// Ok(song) => println!("Do something with analyzed song {} with title {:?}", song.path.display(), song.title), /// Err(e) => println!("Song at {} could not be analyzed. Failed with: {}", path.display(), e), @@ -201,9 +202,9 @@ pub fn analyze_paths, F: IntoIterator>( /// ``` pub fn analyze_paths_with_cores, F: IntoIterator>( paths: F, - number_cores: usize, + number_cores: NonZeroUsize, ) -> mpsc::IntoIter<(PathBuf, BlissResult)> { - let mut cores = num_cpus::get(); + let mut cores = NonZeroUsize::new(num_cpus::get()).unwrap(); if cores > number_cores { cores = number_cores; } @@ -322,7 +323,7 @@ mod tests { assert_eq!(results, expected_results); - let mut results = analyze_paths_with_cores(&paths, 1) + let mut results = analyze_paths_with_cores(&paths, NonZeroUsize::new(1).unwrap()) .map(|x| match &x.1 { Ok(s) => (true, s.path.to_owned(), None), Err(e) => (false, x.0.to_owned(), Some(e.to_string())), diff --git a/src/library.rs b/src/library.rs index a157bb2..97c3dc2 100644 --- a/src/library.rs +++ b/src/library.rs @@ -28,6 +28,7 @@ //! use anyhow::Result; //! use serde::{Deserialize, Serialize}; //! use std::path::PathBuf; +//! use std::num::NonZeroUsize; //! use bliss_audio::BlissError; //! use bliss_audio::library::{AppConfigTrait, BaseConfig}; //! @@ -52,7 +53,7 @@ //! music_library_path: PathBuf, //! config_path: Option, //! database_path: Option, -//! number_cores: Option, +//! number_cores: Option, //! ) -> Result { //! // Note that by passing `(None, None)` here, the paths will //! // be inferred automatically using user data dirs. @@ -107,7 +108,7 @@ //! "real-life" example, the //! [blissify](https://github.com/Polochon-street/blissify-rs)'s code is using //! [Library] to implement bliss for a MPD player. -use crate::analyze_paths; +use crate::analyze_paths_with_cores; use crate::cue::CueInfo; use crate::playlist::closest_album_to_group_by_key; use crate::playlist::closest_to_first_song_by_key; @@ -132,6 +133,7 @@ use std::collections::HashMap; use std::env; use std::fs; use std::fs::create_dir_all; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::Mutex; @@ -167,6 +169,19 @@ pub trait AppConfigTrait: Serialize + Sized + DeserializeOwned { Ok(serde_json::to_string(&self)?) } + /// Set the number of desired cores for analysis, and write it to the + /// configuration file. + fn set_number_cores(&mut self, number_cores: NonZeroUsize) -> Result<()> { + self.base_config_mut().number_cores = number_cores; + self.write() + } + + /// Get the number of desired cores for analysis, and write it to the + /// configuration file. + fn get_number_cores(&self) -> NonZeroUsize { + self.base_config().number_cores + } + /// Default implementation to load a config from a JSON file. /// Reads from a string. /// @@ -215,7 +230,7 @@ pub struct BaseConfig { features_version: u16, /// The number of CPU cores an analysis will be performed with. /// Defaults to the number of CPUs in the user's computer. - number_cores: usize, + number_cores: NonZeroUsize, } impl BaseConfig { @@ -241,7 +256,7 @@ impl BaseConfig { pub fn new( config_path: Option, database_path: Option, - number_cores: Option, + number_cores: Option, ) -> Result { let config_path = { // User provided a path; let the future file creation determine @@ -261,7 +276,8 @@ impl BaseConfig { } }; - let number_cores = number_cores.unwrap_or_else(num_cpus::get); + let number_cores = + number_cores.unwrap_or_else(|| NonZeroUsize::new(num_cpus::get()).unwrap()); Ok(Self { config_path, @@ -449,7 +465,7 @@ impl Library { pub fn new_from_base( config_path: Option, database_path: Option, - number_cores: Option, + number_cores: Option, ) -> Result where BaseConfig: Into, @@ -760,7 +776,10 @@ impl Library { .collect(); let mut cue_extra_info: HashMap = HashMap::new(); - let results = analyze_paths(paths_extra_info.keys()); + let results = analyze_paths_with_cores( + paths_extra_info.keys(), + self.config.base_config().number_cores, + ); let mut success_count = 0; let mut failure_count = 0; for (path, result) in results { @@ -1222,6 +1241,10 @@ mod test { } } + fn nzus(i: usize) -> NonZeroUsize { + NonZeroUsize::new(i).unwrap() + } + // Returning the TempDir here, so it doesn't go out of scope, removing // the directory. // @@ -2787,8 +2810,12 @@ mod test { // In reality, someone would just do that with `(None, None)` to get the default // paths. - let base_config = - BaseConfig::new(Some(config_file.to_owned()), Some(database_file), Some(1)).unwrap(); + let base_config = BaseConfig::new( + Some(config_file.to_owned()), + Some(database_file), + Some(nzus(1)), + ) + .unwrap(); let config = CustomConfig { base_config, @@ -2821,8 +2848,12 @@ mod test { // In reality, someone would just do that with `(None, None)` to get the default // paths. - let base_config = - BaseConfig::new(Some(config_file.to_owned()), Some(database_file), Some(1)).unwrap(); + let base_config = BaseConfig::new( + Some(config_file.to_owned()), + Some(database_file), + Some(nzus(1)), + ) + .unwrap(); let config = CustomConfig { base_config, @@ -2856,6 +2887,39 @@ mod test { assert!(library.version_sanity_check().unwrap()); } + #[test] + fn test_config_number_cpus() { + let config_dir = TempDir::new("coucou").unwrap(); + let config_file = config_dir.path().join("config.json"); + let database_file = config_dir.path().join("bliss.db"); + + let base_config = BaseConfig::new( + Some(config_file.to_owned()), + Some(database_file.to_owned()), + None, + ) + .unwrap(); + let config = CustomConfig { + base_config, + second_path_to_music_library: "/path/to/somewhere".into(), + ignore_wav_files: true, + }; + + assert_eq!(config.get_number_cores().get(), num_cpus::get()); + + let base_config = + BaseConfig::new(Some(config_file), Some(database_file), Some(nzus(1))).unwrap(); + let mut config = CustomConfig { + base_config, + second_path_to_music_library: "/path/to/somewhere".into(), + ignore_wav_files: true, + }; + + assert_eq!(config.get_number_cores().get(), 1); + config.set_number_cores(nzus(2)).unwrap(); + assert_eq!(config.get_number_cores().get(), 2); + } + #[test] fn test_library_create_all_dirs() { let config_dir = TempDir::new("coucou") @@ -2866,7 +2930,7 @@ mod test { assert!(!config_dir.is_dir()); let config_file = config_dir.join("config.json"); let database_file = config_dir.join("bliss.db"); - Library::::new_from_base(Some(config_file), Some(database_file), Some(1)) + Library::::new_from_base(Some(config_file), Some(database_file), Some(nzus(1))) .unwrap(); assert!(config_dir.is_dir()); }