git.iko.soy/helix
on master help sign in
Picker performance improvements
78fba86Blaž Hrastnik
Cargo.lock+7
compare with master
unifiedsplit
@@ -449,6 +449,7 @@ dependencies = [
449 "num_cpus",
449 "num_cpus",
450 "once_cell",
450 "once_cell",
451 "pulldown-cmark",
451 "pulldown-cmark",
452+ "retain_mut",
452 "serde",
453 "serde",
453 "serde_json",
454 "serde_json",
454 "signal-hook",
455 "signal-hook",
@@ -845,6 +846,12 @@ version = "0.6.25"
845source = "registry+https://github.com/rust-lang/crates.io-index"
846source = "registry+https://github.com/rust-lang/crates.io-index"
846checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b"
847checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b"
847
848
849+[[package]]
850+name = "retain_mut"
851+version = "0.1.7"
852+source = "registry+https://github.com/rust-lang/crates.io-index"
853+checksum = "8c31b5c4033f8fdde8700e4657be2c497e7288f01515be52168c631e2e4d4086"
854+
848[[package]]
855[[package]]
849name = "ropey"
856name = "ropey"
850version = "1.3.2"
857version = "1.3.2"
helix-term/Cargo.toml+3
@@ -61,5 +61,8 @@ serde = { version = "1.0", features = ["derive"] }
61grep-regex = "0.1.9"
61grep-regex = "0.1.9"
62grep-searcher = "0.1.8"
62grep-searcher = "0.1.8"
63
63
64+# Remove once retain_mut lands in stable rust
65+retain_mut = "0.1.7"
66+
64[target.'cfg(not(windows))'.dependencies] # https://github.com/vorner/signal-hook/issues/100
67[target.'cfg(not(windows))'.dependencies] # https://github.com/vorner/signal-hook/issues/100
65signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] }
68signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] }
helix-term/src/ui/mod.rs+17−27
@@ -98,7 +98,9 @@ pub fn regex_prompt(
98
98
99pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePicker<PathBuf> {
99pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePicker<PathBuf> {
100 use ignore::{types::TypesBuilder, WalkBuilder};
100 use ignore::{types::TypesBuilder, WalkBuilder};
101− use std::time;
101+ use std::time::Instant;
102+
103+ let now = Instant::now();
102
104
103 let mut walk_builder = WalkBuilder::new(&root);
105 let mut walk_builder = WalkBuilder::new(&root);
104 walk_builder
106 walk_builder
@@ -116,56 +118,44 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi
116
118
117 // We want to exclude files that the editor can't handle yet
119 // We want to exclude files that the editor can't handle yet
118 let mut type_builder = TypesBuilder::new();
120 let mut type_builder = TypesBuilder::new();
119−
120 type_builder
121 type_builder
121 .add(
122 .add(
122 "compressed",
123 "compressed",
123 "*.{zip,gz,bz2,zst,lzo,sz,tgz,tbz2,lz,lz4,lzma,lzo,z,Z,xz,7z,rar,cab}",
124 "*.{zip,gz,bz2,zst,lzo,sz,tgz,tbz2,lz,lz4,lzma,lzo,z,Z,xz,7z,rar,cab}",
124 )
125 )
125− // This shouldn't panic as the above is static, but if it ever
126− // is changed and becomes invalid it will catch here rather than
127− // being unnoticed.
128 .expect("Invalid type definition");
126 .expect("Invalid type definition");
129 type_builder.negate("all");
127 type_builder.negate("all");
130−
131− if let Ok(excluded_types) = type_builder.build() {
128+ let excluded_types = type_builder
129+ .build()
130+ .expect("failed to build excluded_types");
132− walk_builder.types(excluded_types);
131+ walk_builder.types(excluded_types);
133− }
134
132
135 // We want files along with their modification date for sorting
133 // We want files along with their modification date for sorting
136 let files = walk_builder.build().filter_map(|entry| {
134 let files = walk_builder.build().filter_map(|entry| {
137 let entry = entry.ok()?;
135 let entry = entry.ok()?;
138− // Path::is_dir() traverses symlinks, so we use it over DirEntry::is_dir
136+
137+ // This is faster than entry.path().is_dir() since it uses cached fs::Metadata fetched by ignore/walkdir
138+ let is_dir = entry.file_type().map(|ft| ft.is_dir()).unwrap_or(false);
139+
139− if entry.path().is_dir() {
140+ if is_dir {
140 // Will give a false positive if metadata cannot be read (eg. permission error)
141 // Will give a false positive if metadata cannot be read (eg. permission error)
141 return None;
142 return None;
142 }
143 }
143
144
144− let time = entry.metadata().map_or(time::UNIX_EPOCH, |metadata| {
145− metadata
146− .accessed()
147− .or_else(|_| metadata.modified())
148− .or_else(|_| metadata.created())
149− .unwrap_or(time::UNIX_EPOCH)
150− });
151−
152− Some((entry.into_path(), time))
145+ Some(entry.into_path())
153 });
146 });
154
147
155 // Cap the number of files if we aren't in a git project, preventing
148 // Cap the number of files if we aren't in a git project, preventing
156 // hangs when using the picker in your home directory
149 // hangs when using the picker in your home directory
157− let mut files: Vec<_> = if root.join(".git").is_dir() {
150+ let files: Vec<_> = if root.join(".git").is_dir() {
158 files.collect()
151 files.collect()
159 } else {
152 } else {
160− const MAX: usize = 8192;
153+ // const MAX: usize = 8192;
154+ const MAX: usize = 100_000;
161 files.take(MAX).collect()
155 files.take(MAX).collect()
162 };
156 };
163
157
164− // Most recently modified first
158+ log::debug!("file_picker init {:?}", Instant::now().duration_since(now));
165− files.sort_by_key(|file| std::cmp::Reverse(file.1));
166−
167− // Strip the time data so we can send just the paths to the FilePicker
168− let files = files.into_iter().map(|(path, _)| path).collect();
169
159
170 FilePicker::new(
160 FilePicker::new(
171 files,
161 files,
helix-term/src/ui/picker.rs+81−25
@@ -13,8 +13,10 @@ use fuzzy_matcher::skim::SkimMatcherV2 as Matcher;
13use fuzzy_matcher::FuzzyMatcher;
13use fuzzy_matcher::FuzzyMatcher;
14use tui::widgets::Widget;
14use tui::widgets::Widget;
15
15
16+use std::time::Instant;
16use std::{
17use std::{
17 borrow::Cow,
18 borrow::Cow,
19+ cmp::Reverse,
18 collections::HashMap,
20 collections::HashMap,
19 io::Read,
21 io::Read,
20 path::{Path, PathBuf},
22 path::{Path, PathBuf},
@@ -286,7 +288,8 @@ pub struct Picker<T> {
286 cursor: usize,
288 cursor: usize,
287 // pattern: String,
289 // pattern: String,
288 prompt: Prompt,
290 prompt: Prompt,
291+ previous_pattern: String,
289− /// Wheather to truncate the start (default true)
292+ /// Whether to truncate the start (default true)
290 pub truncate_start: bool,
293 pub truncate_start: bool,
291
294
292 format_fn: Box<dyn Fn(&T) -> Cow<str>>,
295 format_fn: Box<dyn Fn(&T) -> Cow<str>>,
@@ -303,9 +306,7 @@ impl<T> Picker<T> {
303 "".into(),
306 "".into(),
304 None,
307 None,
305 ui::completers::none,
308 ui::completers::none,
306− |_editor: &mut Context, _pattern: &str, _event: PromptEvent| {
309+ |_editor: &mut Context, _pattern: &str, _event: PromptEvent| {},
307− //
308− },
309 );
310 );
310
311
311 let mut picker = Self {
312 let mut picker = Self {
@@ -315,44 +316,99 @@ impl<T> Picker<T> {
315 filters: Vec::new(),
316 filters: Vec::new(),
316 cursor: 0,
317 cursor: 0,
317 prompt,
318 prompt,
319+ previous_pattern: String::new(),
318 truncate_start: true,
320 truncate_start: true,
319 format_fn: Box::new(format_fn),
321 format_fn: Box::new(format_fn),
320 callback_fn: Box::new(callback_fn),
322 callback_fn: Box::new(callback_fn),
321 completion_height: 0,
323 completion_height: 0,
322 };
324 };
323
325
324− // TODO: scoring on empty input should just use a fastpath
326+ // scoring on empty input:
325− picker.score();
327+ // TODO: just reuse score()
328+ picker.matches.extend(
329+ picker
330+ .options
331+ .iter()
332+ .enumerate()
333+ .map(|(index, _option)| (index, 0)),
334+ );
326
335
327 picker
336 picker
328 }
337 }
329
338
330 pub fn score(&mut self) {
339 pub fn score(&mut self) {
340+ let now = Instant::now();
341+
331 let pattern = &self.prompt.line;
342 let pattern = &self.prompt.line;
332
343
333− // reuse the matches allocation
344+ if pattern == &self.previous_pattern {
345+ return;
346+ }
347+
348+ if pattern.is_empty() {
349+ // Fast path for no pattern.
334− self.matches.clear();
350+ self.matches.clear();
335− self.matches.extend(
351+ self.matches.extend(
336− self.options
352+ self.options
337− .iter()
353+ .iter()
338− .enumerate()
354+ .enumerate()
339− .filter_map(|(index, option)| {
355+ .map(|(index, _option)| (index, 0)),
340− // filter options first before matching
356+ );
341− if !self.filters.is_empty() {
357+ } else if pattern.starts_with(&self.previous_pattern) {
342− self.filters.binary_search(&index).ok()?;
358+ // TODO: remove when retain_mut is in stable rust
359+ use retain_mut::RetainMut;
360+
361+ // optimization: if the pattern is a more specific version of the previous one
362+ // then we can score the filtered set.
363+ #[allow(unstable_name_collisions)]
364+ self.matches.retain_mut(|(index, score)| {
365+ let option = &self.options[*index];
366+ // TODO: maybe using format_fn isn't the best idea here
367+ let text = (self.format_fn)(option);
368+
369+ match self.matcher.fuzzy_match(&text, pattern) {
370+ Some(s) => {
371+ // Update the score
372+ *score = s;
373+ true
343 }
374 }
375+ None => false,
376+ }
377+ });
378+
379+ self.matches
380+ .sort_unstable_by_key(|(_, score)| Reverse(*score));
381+ } else {
382+ self.matches.clear();
383+ self.matches.extend(
384+ self.options
385+ .iter()
386+ .enumerate()
387+ .filter_map(|(index, option)| {
388+ // filter options first before matching
389+ if !self.filters.is_empty() {
390+ // TODO: this filters functionality seems inefficient,
391+ // instead store and operate on filters if any
392+ self.filters.binary_search(&index).ok()?;
393+ }
394+
344− // TODO: maybe using format_fn isn't the best idea here
395+ // TODO: maybe using format_fn isn't the best idea here
345− let text = (self.format_fn)(option);
396+ let text = (self.format_fn)(option);
346− // Highlight indices are computed lazily in the render function
397+
347− self.matcher
398+ self.matcher
348− .fuzzy_match(&text, pattern)
399+ .fuzzy_match(&text, pattern)
349− .map(|score| (index, score))
400+ .map(|score| (index, score))
350− }),
401+ }),
351− );
402+ );
403+ self.matches
352− self.matches.sort_unstable_by_key(|(_, score)| -score);
404+ .sort_unstable_by_key(|(_, score)| Reverse(*score));
405+ }
406+
407+ log::debug!("picker score {:?}", Instant::now().duration_since(now));
353
408
354 // reset cursor position
409 // reset cursor position
355 self.cursor = 0;
410 self.cursor = 0;
411+ self.previous_pattern.clone_from(pattern);
356 }
412 }
357
413
358 /// Move the cursor by a number of lines, either down (`Forward`) or up (`Backward`)
414 /// Move the cursor by a number of lines, either down (`Forward`) or up (`Backward`)