diff --git a/Cargo.lock b/Cargo.lock index 0d0aa567..a4971517 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,13 +4,14 @@ version = 3 [[package]] name = "ahash" -version = "0.8.3" +version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" +checksum = "91429305e9f0a25f6205c5b8e0d2db09e0708a7a6df0f42212bb56c32c8ac97a" dependencies = [ "cfg-if", "once_cell", "version_check", + "zerocopy", ] [[package]] @@ -234,6 +235,12 @@ version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + [[package]] name = "errno" version = "0.3.1" @@ -313,9 +320,9 @@ checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" [[package]] name = "hashbrown" -version = "0.14.0" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c6201b9ff9fd90a5a3bac2e56a830d0caa509576f0e503818ee82c181b3437a" +checksum = "290f1a1d9242c78d09ce40a5e87e7554ee637af1351968159f4952f028f75604" dependencies = [ "ahash", "allocator-api2", @@ -359,6 +366,16 @@ dependencies = [ "cc", ] +[[package]] +name = "indexmap" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d530e1a18b1cb4c484e6e34556a0d948706958449fca0cab753d649f2bce3d1f" +dependencies = [ + "equivalent", + "hashbrown", +] + [[package]] name = "itertools" version = "0.10.5" @@ -605,6 +622,7 @@ dependencies = [ "crossterm", "fd-lock", "gethostname", + "indexmap", "itertools", "nu-ansi-term", "pretty_assertions", @@ -841,9 +859,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.26" +version = "2.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45c3457aacde3c65315de5031ec191ce46604304d2446e803d71ade03308d970" +checksum = "239814284fd6f1a4ffe4ca893952cdd93c224b6a1571c9a9eadd670295c0c9e2" dependencies = [ "proc-macro2", "quote", @@ -1120,3 +1138,23 @@ name = "yansi" version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" + +[[package]] +name = "zerocopy" +version = "0.7.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d075cf85bbb114e933343e087b92f2146bac0d55b534cbb8188becf0039948e" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.7.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86cd5ca076997b97ef09d3ad65efe811fa68c9e874cb636ccb211223a813b0c2" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] diff --git a/Cargo.toml b/Cargo.toml index 0d3a7683..ff7633d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ clipboard = { version = "0.5.0", optional = true } crossbeam = { version = "0.8.2", optional = true } crossterm = { version = "0.27.0", features = ["serde"] } fd-lock = "3.0.3" +indexmap = "2.1.0" itertools = "0.10.3" nu-ansi-term = "0.49.0" rand = { version = "0.8.5", default-features = false, features = [ diff --git a/src/history/file_backed.rs b/src/history/file_backed.rs index 6b11f9e3..7fc73876 100644 --- a/src/history/file_backed.rs +++ b/src/history/file_backed.rs @@ -1,3 +1,5 @@ +use indexmap::IndexMap; + use super::{ base::CommandLineSearch, History, HistoryItem, HistoryItemId, SearchDirection, SearchQuery, }; @@ -7,7 +9,6 @@ use crate::{ }; use std::{ - collections::VecDeque, fs::OpenOptions, io::{BufRead, BufReader, BufWriter, Seek, SeekFrom, Write}, ops::{Deref, DerefMut}, @@ -27,9 +28,9 @@ pub const NEWLINE_ESCAPE: &str = "<\\n>"; #[derive(Debug)] pub struct FileBackedHistory { capacity: usize, - entries: VecDeque, + entries: IndexMap, file: Option, - len_on_disk: usize, // Keep track what was previously written to disk + last_on_disk: Option, session: Option, } @@ -42,12 +43,19 @@ impl Default for FileBackedHistory { } } -fn encode_entry(s: &str) -> String { - s.replace('\n', NEWLINE_ESCAPE) +fn encode_entry(id: HistoryItemId, s: &str) -> String { + format!("{id}:{}", s.replace('\n', NEWLINE_ESCAPE)) } -fn decode_entry(s: &str) -> String { - s.replace(NEWLINE_ESCAPE, "\n") +fn decode_entry(s: &str) -> std::result::Result<(HistoryItemId, String), &'static str> { + let sep = s + .bytes() + .position(|b| b == b':') + .ok_or("History item ID is missing")?; + + let id = s[..sep].parse().map_err(|_| "Invalid history item ID")?; + + Ok((HistoryItemId(id), s.replace(NEWLINE_ESCAPE, "\n"))) } impl History for FileBackedHistory { @@ -62,17 +70,17 @@ impl History for FileBackedHistory { // Don't append if the preceding value is identical or the string empty if self .entries - .back() - .map_or(true, |previous| previous != &entry) + .last() + .map_or(true, |(_, previous)| previous != &entry) && !entry.is_empty() { if self.entries.len() == self.capacity { // History is "full", so we delete the oldest entry first, // before adding a new one. - self.entries.pop_front(); - self.len_on_disk = self.len_on_disk.saturating_sub(1); + self.entries.remove(&HistoryItemId(0)); } - self.entries.push_back(entry.to_string()); + + self.entries.insert(h.id, entry.to_string()); } Ok(()) @@ -87,7 +95,7 @@ impl History for FileBackedHistory { Ok(FileBackedHistory::construct_entry( id, self.entries - .get(id.0 as usize) + .get(&id) .ok_or(ReedlineError(ReedlineErrorVariants::OtherHistoryError( "Item does not exist", )))? @@ -122,31 +130,19 @@ impl History for FileBackedHistory { }, )); } - let (min_id, max_id) = { - let start = query.start_id.map(|e| e.0); - let end = query.end_id.map(|e| e.0); + + let (from_id, to_id) = { + let start = query.start_id; + let end = query.end_id; + if let SearchDirection::Backward = query.direction { (end, start) } else { (start, end) } }; - // add one to make it inclusive - let min_id = min_id.map(|e| e + 1).unwrap_or(0); - // subtract one to make it inclusive - let max_id = max_id - .map(|e| e - 1) - .unwrap_or(self.entries.len() as i64 - 1); - if max_id < 0 || min_id > self.entries.len() as i64 - 1 { - return Ok(vec![]); - } - let intrinsic_limit = max_id - min_id + 1; - let limit = if let Some(given_limit) = query.limit { - std::cmp::min(intrinsic_limit, given_limit) as usize - } else { - intrinsic_limit as usize - }; - let filter = |(idx, cmd): (usize, &String)| { + + let filter = |(_, (id, cmd)): (usize, (&HistoryItemId, &String))| { if !match &query.filter.command_line { Some(CommandLineSearch::Prefix(p)) => cmd.starts_with(p), Some(CommandLineSearch::Substring(p)) => cmd.contains(p), @@ -161,17 +157,37 @@ impl History for FileBackedHistory { } } Some(FileBackedHistory::construct_entry( - HistoryItemId::new(idx as i64), + *id, cmd.to_string(), // todo: this copy might be a perf bottleneck )) }; + let from_index = match from_id { + Some(from_id) => self.entries.binary_search_keys(&from_id).expect("todo"), + None => 0, + }; + + let to_index = match to_id { + Some(to_id) => self.entries.binary_search_keys(&to_id).expect("todo"), + None => self.entries.len().saturating_sub(1), + }; + + if from_index >= to_index { + return Ok(vec![]); + } + let iter = self .entries .iter() .enumerate() - .skip(min_id as usize) - .take(intrinsic_limit as usize); + .skip(from_index) + .take(to_index - from_index); + + let limit = match query.limit { + Some(limit) => usize::try_from(limit).unwrap(), + None => usize::MAX, + }; + if let SearchDirection::Backward = query.direction { Ok(iter.rev().filter_map(filter).take(limit).collect()) } else { @@ -194,7 +210,7 @@ impl History for FileBackedHistory { fn clear(&mut self) -> Result<()> { self.entries.clear(); - self.len_on_disk = 0; + self.last_on_disk = None; if let Some(file) = &self.file { if let Err(err) = std::fs::remove_file(file) { @@ -220,7 +236,16 @@ impl History for FileBackedHistory { fn sync(&mut self) -> std::io::Result<()> { if let Some(fname) = &self.file { // The unwritten entries - let own_entries = self.entries.range(self.len_on_disk..); + let last_index_on_disk = match self.last_on_disk { + Some(last_id) => self.entries.binary_search_keys(&last_id).unwrap(), + None => 0, + }; + + if last_index_on_disk == self.entries.len() { + return Ok(()); + } + + let own_entries = self.entries.get_range(last_index_on_disk..).unwrap(); if let Some(base_dir) = fname.parent() { std::fs::create_dir_all(base_dir)?; @@ -234,12 +259,15 @@ impl History for FileBackedHistory { .open(fname)?, ); let mut writer_guard = f_lock.write()?; + let (mut foreign_entries, truncate) = { let reader = BufReader::new(writer_guard.deref()); + let mut from_file = reader .lines() - .map(|o| o.map(|i| decode_entry(&i))) - .collect::>>()?; + .map(|o| o.map(|i| decode_entry(&i).expect("todo: error handling"))) + .collect::>>()?; + if from_file.len() + own_entries.len() > self.capacity { ( from_file.split_off(from_file.len() - (self.capacity - own_entries.len())), @@ -252,33 +280,38 @@ impl History for FileBackedHistory { { let mut writer = BufWriter::new(writer_guard.deref_mut()); + if truncate { writer.rewind()?; - for line in &foreign_entries { - writer.write_all(encode_entry(line).as_bytes())?; + for (id, line) in &foreign_entries { + writer.write_all(encode_entry(*id, line).as_bytes())?; writer.write_all("\n".as_bytes())?; } } else { writer.seek(SeekFrom::End(0))?; } - for line in own_entries { - writer.write_all(encode_entry(line).as_bytes())?; + + for (id, line) in own_entries { + writer.write_all(encode_entry(*id, line).as_bytes())?; writer.write_all("\n".as_bytes())?; } + writer.flush()?; } + if truncate { let file = writer_guard.deref_mut(); let file_len = file.stream_position()?; file.set_len(file_len)?; } - let own_entries = self.entries.drain(self.len_on_disk..); + let own_entries = self.entries.drain(last_index_on_disk + 1..); foreign_entries.extend(own_entries); self.entries = foreign_entries; - self.len_on_disk = self.entries.len(); + let last_entry = self.entries.last().unwrap(); + self.last_on_disk = Some(*last_entry.0); } Ok(()) } @@ -300,9 +333,9 @@ impl FileBackedHistory { } FileBackedHistory { capacity, - entries: VecDeque::new(), + entries: IndexMap::new(), file: None, - len_on_disk: 0, + last_on_disk: None, session: None, } }