From 127124eded17711de024db5bc781ddd6f7bf4276 Mon Sep 17 00:00:00 2001 From: Danilo Bargen Date: Fri, 29 Jan 2021 01:25:06 +0100 Subject: [PATCH] Optimize get_languages function - Replace `Result` parameters with `Option` - Replace `HashSet` for deduplication with linear search based approach - Avoid intermediate allocations This reduces the instruction count in release mode by almost 50%. --- src/cache.rs | 2 +- src/dedup.rs | 22 ++++++++++++++++ src/main.rs | 71 +++++++++++++++++++++++++--------------------------- 3 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 src/dedup.rs diff --git a/src/cache.rs b/src/cache.rs index 9023aaf..0faddf4 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -159,7 +159,7 @@ impl Cache { pub fn find_page(&self, name: &str, languages: &[String]) -> Option { let page_filename = format!("{}.md", name); - // Get platform dir + // Get cache dir let cache_dir = match Self::get_cache_dir() { Ok(cache_dir) => cache_dir.join("tldr-master"), Err(e) => { diff --git a/src/dedup.rs b/src/dedup.rs new file mode 100644 index 0000000..b196efa --- /dev/null +++ b/src/dedup.rs @@ -0,0 +1,22 @@ +/// An extension trait to clear duplicates from a collection. +pub(crate) trait Dedup { + fn clear_duplicates(&mut self); +} + +/// Clear duplicates from a collection, keep the first one seen. +/// +/// For small vectors, this will be faster than a `HashSet`. +/// Based on +impl Dedup for Vec { + fn clear_duplicates(&mut self) { + let mut already_seen = Vec::with_capacity(self.len()); + self.retain(|item| { + if already_seen.contains(item) { + false + } else { + already_seen.push(item.clone()); + true + } + }) + } +} diff --git a/src/main.rs b/src/main.rs index ac874f2..ef3e2aa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,11 +17,11 @@ #[cfg(feature = "logging")] extern crate env_logger; -use std::collections::HashSet; use std::env; use std::fs::File; use std::io::BufRead; use std::io::BufReader; +use std::iter; use std::path::{Path, PathBuf}; use std::process; @@ -35,6 +35,7 @@ use serde_derive::Deserialize; mod cache; mod config; +mod dedup; mod error; mod formatter; mod tokenizer; @@ -42,6 +43,7 @@ mod types; use crate::cache::Cache; use crate::config::{get_config_path, make_default_config, Config, MAX_CACHE_AGE}; +use crate::dedup::Dedup; use crate::error::TealdeerError::{CacheError, ConfigError, UpdateError}; use crate::formatter::print_lines; use crate::tokenizer::Tokenizer; @@ -257,44 +259,42 @@ fn get_os() -> OsType { OsType::Other } -fn get_languages( - env_lang: Result, - env_language: Result, -) -> Vec { +fn get_languages(env_lang: Option, env_language: Option) -> Vec { // Language list according to // https://github.com/tldr-pages/tldr/blob/master/CLIENT-SPECIFICATION.md#language - if let Ok(lang) = env_lang { - let language = env_language.unwrap_or_default(); - let mut locales: Vec<&str> = language.split(':').collect(); - locales.push(&lang); - locales.push("en"); + if let Some(lang) = env_lang { + // Create an iterator that contains $LANGUAGES, split by `:`, then followed by $LANG. + let locales = env_language + .as_deref() + .unwrap_or("") + .split(':') + .chain(iter::once(&*lang)); let mut lang_list = Vec::new(); - let mut found_languages = HashSet::new(); - - for locale in &locales { + for locale in locales { + // Language plus country code (e.g. `en_US`) if locale.len() >= 5 && locale.chars().nth(2) == Some('_') { - // Language with country code - let lang = &locale[..5]; - if found_languages.insert(lang) { - lang_list.push(lang); - } + lang_list.push(&locale[..5]); } - if locale.len() >= 2 && *locale != "POSIX" { - // Language code - let lang = &locale[..2]; - if found_languages.insert(lang) { - lang_list.push(lang); - } + // Language code only (e.g. `en`) + if locale.len() >= 2 && locale != "POSIX" { + lang_list.push(&locale[..2]); } } + // Fallback language + lang_list.push("en"); + + // Deduplicate entries + lang_list.clear_duplicates(); + + // Convert Vec<&str> into Vec return lang_list.iter().map(|&s| String::from(s)).collect(); } // Without the LANG environment variable, only English pages should be looked up. - vec!["en".into()] + vec!["en".to_string()] } fn main() { @@ -428,7 +428,7 @@ fn main() { // Language overwritten by console argument vec![lang.clone()] } else { - get_languages(std::env::var("LANG"), std::env::var("LANGUAGE")) + get_languages(std::env::var("LANG").ok(), std::env::var("LANGUAGE").ok()) }; // Search for command in cache @@ -483,44 +483,41 @@ mod test { #[test] fn missing_lang_env() { - let lang_list = get_languages(Err(std::env::VarError::NotPresent), Ok("de:fr".into())); + let lang_list = get_languages(None, Some("de:fr".into())); assert_eq!(lang_list, vec!["en"]); - let lang_list = get_languages( - Err(std::env::VarError::NotPresent), - Err(std::env::VarError::NotPresent), - ); + let lang_list = get_languages(None, None); assert_eq!(lang_list, vec!["en"]); } #[test] fn missing_language_env() { - let lang_list = get_languages(Ok("de".into()), Err(std::env::VarError::NotPresent)); + let lang_list = get_languages(Some("de".into()), None); assert_eq!(lang_list, vec!["de", "en"]); } #[test] fn preference_order() { - let lang_list = get_languages(Ok("de".into()), Ok("fr:cn".into())); + let lang_list = get_languages(Some("de".into()), Some("fr:cn".into())); assert_eq!(lang_list, vec!["fr", "cn", "de", "en"]); } #[test] fn country_code_expansion() { - let lang_list = get_languages(Ok("pt_BR".into()), Err(std::env::VarError::NotPresent)); + let lang_list = get_languages(Some("pt_BR".into()), None); assert_eq!(lang_list, vec!["pt_BR", "pt", "en"]); } #[test] fn ignore_posix_and_c() { - let lang_list = get_languages(Ok("POSIX".into()), Err(std::env::VarError::NotPresent)); + let lang_list = get_languages(Some("POSIX".into()), None); assert_eq!(lang_list, vec!["en"]); - let lang_list = get_languages(Ok("C".into()), Err(std::env::VarError::NotPresent)); + let lang_list = get_languages(Some("C".into()), None); assert_eq!(lang_list, vec!["en"]); } #[test] fn no_duplicates() { - let lang_list = get_languages(Ok("de".into()), Ok("fr:de:cn:de".into())); + let lang_list = get_languages(Some("de".into()), Some("fr:de:cn:de".into())); assert_eq!(lang_list, vec!["fr", "de", "cn", "en"]); } }