File picker: allow parallel traversal w/o sorting #12688
Conversation
Motivated by the discussion in helix-editor#10995. Sequential traversal is usually very fast, unless it's Windows, or the files are on a (slow) remotely mounted FS. The existing logic of *not* moving injection of file names to a background thread is preserved: if a directory can be traversed in 30ms, all the work happens on the same thread sequentially. To use `WalkParallel`, we need to give up on file name sorting, so add a file picker option for that (defaults to sorting enabled to not change the current behaviour). We _could_ make sorting work for parallel traversal as well, but that requires eagerly traversing the file tree and fully realizing the collection of file paths in memory (so that we can merge partial results from independent workers). It looks like lazy evaluation is preferred and allows us to stop the work early (e.g. if the file picker window is closed). Closes helix-editor#11021.
| } | ||
| Some(entry.into_path()) | ||
| }); | ||
| log::debug!("file_picker init {:?}", Instant::now().duration_since(now)); |
There was a problem hiding this comment.
My understanding is that this log predates lazy traversal of the file tree. As it was, it would record how long it takes to build an instance of Walk, but, I think, we are more interested in how long it takes to traverse the file tree and inject all file entries into a Picker?
|
|
||
| // How long we are willing to wait for results before displaying the file picker. | ||
| let now = Instant::now(); | ||
| let timeout = now + Duration::from_millis(30); |
There was a problem hiding this comment.
This would probably be a config option as well. Otherwise, it's unclear how to reliably trigger all code paths from the integration tests.
| @@ -268,15 +285,49 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi | |||
| } | |||
| } | |||
| if hit_timeout { | |||
There was a problem hiding this comment.
A possible alternative would be to drop this timeout to build and return an (empty) Picker as fast as possible, and always populate it from a background thread. I'm not entirely sure why this behaviour is preferred. On my Linux machine, the delay of spawning and scheduling a new thread is a few orders of magnitude lower than the current timeout duration, but, perhaps, on other platforms it is preferable to avoid creating a new thread if traversing the file tree is faster than what a human perceives to be a slow UI.
|
Opening a PR to get some high level feedback if you think the suggested approach makes sense. Happy to add integration tests if you agree! |
Motivated by the discussion in #10995. Sequential traversal is usually very fast, unless it's Windows, or the files are on a (slow) remotely mounted FS.
The existing logic of not moving injection of file names to a background thread is preserved: if a directory can be traversed in 30ms, all the work happens on the same thread sequentially.
To use
WalkParallel, we need to give up on file name sorting, so add a file picker option for that (defaults to sorting enabled to not change the current behaviour). We could make sorting work for parallel traversal as well, but that requires eagerly traversing the file tree and fully realizing the collection of file paths in memory (so that we can merge partial results from independent workers). It looks like lazy evaluation is preferred and allows us to stop the work early (e.g. if the file picker window is closed).Closes #11021.