detect remote filesystems so SFTP browsing is not slow (#1996)
- [x] I have disclosed use of any AI generated code in my commit messages. (didnt use any) - If you are using an LLM, and do not fully understand the changes it is making to the code base, do not create a PR. - In our experience, AI generated code often results in overly complex code that lacks enough context for a proper fix or feature inclusion. This results in considerably longer code reviews. Due to this, AI authored or partially authored PRs may be closed without comment. - [x] I understand these changes in full and will be able to respond to review comments. - [x] My change is accurately described in the commit message. - [x] My contribution is tested and working as described. - [x] I have read the [Developer Certificate of Origin](https://developercertificate.org/) and certify my contribution under its conditions. Fixes #582 filesystem::remote only comes from query_filesystem_info(), but both scan paths read it from enumerate_children(), where it is always missing. Every remote file was treated as local, so each guard that skips expensive work on remote filesystems did nothing. Also request only the attributes actually used instead of "*", add is_dir to ItemMetadata::GvfsPath, and count children in the background for visible items.
This commit is contained in:
Jeremy Soller
GitHub
commit
9642fef9cd
2 changed files with 179 additions and 37 deletions
|
|
@ -18,6 +18,15 @@ use crate::tab::{self, ChecksumState, DirSize, ItemMetadata, ItemThumbnail, Loca
|
|||
|
||||
const TARGET_URI_ATTRIBUTE: &str = "standard::target-uri";
|
||||
|
||||
// Attributes requested when listing a network directory.
|
||||
const SCAN_ATTRIBUTES: &str = "standard::name,\
|
||||
standard::display-name,\
|
||||
standard::type,\
|
||||
standard::size,\
|
||||
standard::icon,\
|
||||
standard::is-hidden,\
|
||||
time::modified";
|
||||
|
||||
fn resolve_uri(uri: &str) -> (String, gio::File) {
|
||||
let file = gio::File::for_uri(uri);
|
||||
// Resolve the target-uri if it exists
|
||||
|
|
@ -119,9 +128,27 @@ fn network_scan(uri: &str, sizes: IconSizes) -> Result<Vec<tab::Item>, String> {
|
|||
Box::from([])
|
||||
};
|
||||
|
||||
// `filesystem::remote` belongs to the filesystem namespace, which `enumerate_children`
|
||||
// never fills in, so it has to be queried once for the directory being listed.
|
||||
let remote = file
|
||||
.query_filesystem_info(
|
||||
gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE,
|
||||
gio::Cancellable::NONE,
|
||||
)
|
||||
.map(|info| info.boolean(gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE))
|
||||
.unwrap_or_else(|err| {
|
||||
log::warn!("failed to get GIO filesystem info for {uri}: {err}");
|
||||
// Assume remote, so per-entry work is skipped rather than retried
|
||||
true
|
||||
});
|
||||
|
||||
let mut items = Vec::new();
|
||||
for info_res in file
|
||||
.enumerate_children("*", gio::FileQueryInfoFlags::NONE, gio::Cancellable::NONE)
|
||||
.enumerate_children(
|
||||
SCAN_ATTRIBUTES,
|
||||
gio::FileQueryInfoFlags::NONE,
|
||||
gio::Cancellable::NONE,
|
||||
)
|
||||
.map_err(err_str)?
|
||||
{
|
||||
let info = info_res.map_err(err_str)?;
|
||||
|
|
@ -133,13 +160,17 @@ fn network_scan(uri: &str, sizes: IconSizes) -> Result<Vec<tab::Item>, String> {
|
|||
//TODO: what is the best way to resolve shortcuts?
|
||||
let location = Location::Network(uri, display_name.clone(), file.child(&name).path());
|
||||
|
||||
let metadata = if !force_dir && !info.boolean(gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE) {
|
||||
let metadata = if force_dir {
|
||||
ItemMetadata::SimpleDir { entries: 0 }
|
||||
} else {
|
||||
let mtime = info.attribute_uint64(gio::FILE_ATTRIBUTE_TIME_MODIFIED);
|
||||
let is_dir = matches!(info.file_type(), gio::FileType::Directory);
|
||||
let size_opt = (!is_dir).then_some(info.size() as u64);
|
||||
let mut children_opt = None;
|
||||
|
||||
if is_dir {
|
||||
// Counting children costs a directory listing per entry, which is far too
|
||||
// expensive on a remote filesystem
|
||||
if is_dir && !remote {
|
||||
if let Some(path) = file.child(&name).path() {
|
||||
//TODO: calculate children in the background (and make it cancellable?)
|
||||
match std::fs::read_dir(&path) {
|
||||
|
|
@ -159,9 +190,8 @@ fn network_scan(uri: &str, sizes: IconSizes) -> Result<Vec<tab::Item>, String> {
|
|||
mtime,
|
||||
size_opt,
|
||||
children_opt,
|
||||
is_dir,
|
||||
}
|
||||
} else {
|
||||
ItemMetadata::SimpleDir { entries: 0 }
|
||||
};
|
||||
|
||||
let (mime, icon_handle_grid, icon_handle_list, icon_handle_list_condensed) = {
|
||||
|
|
|
|||
176
src/tab.rs
176
src/tab.rs
|
|
@ -642,6 +642,41 @@ fn display_name_for_file(path: &Path, name: &str, get_from_gvfs: bool, is_deskto
|
|||
Item::display_name(name)
|
||||
}
|
||||
|
||||
// Whether a dir lives on a remote filesystem, according to GIO.
|
||||
#[cfg(feature = "gvfs")]
|
||||
fn gvfs_dir_is_remote(dir: &Path) -> bool {
|
||||
static REMOTE_CACHE: LazyLock<RwLock<FxHashMap<PathBuf, bool>>> =
|
||||
LazyLock::new(|| RwLock::new(FxHashMap::default()));
|
||||
|
||||
if let Some(remote) = REMOTE_CACHE.read().unwrap().get(dir) {
|
||||
return *remote;
|
||||
}
|
||||
|
||||
let remote = match gio::prelude::FileExt::query_filesystem_info(
|
||||
&gio::File::for_path(dir),
|
||||
gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE,
|
||||
gio::Cancellable::NONE,
|
||||
) {
|
||||
Ok(info) => info.boolean(gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE),
|
||||
Err(err) => {
|
||||
log::warn!(
|
||||
"failed to get GIO filesystem info for {}: {}",
|
||||
dir.display(),
|
||||
err
|
||||
);
|
||||
|
||||
//Assume remote so that the "expensive tasks" are rather skipped then actually executed.
|
||||
true
|
||||
}
|
||||
};
|
||||
|
||||
REMOTE_CACHE
|
||||
.write()
|
||||
.unwrap()
|
||||
.insert(dir.to_path_buf(), remote);
|
||||
remote
|
||||
}
|
||||
|
||||
#[cfg(feature = "gvfs")]
|
||||
pub fn item_from_gvfs_info(path: PathBuf, file_info: gio::FileInfo, sizes: IconSizes) -> Item {
|
||||
let file_name = file_info
|
||||
|
|
@ -649,7 +684,7 @@ pub fn item_from_gvfs_info(path: PathBuf, file_info: gio::FileInfo, sizes: IconS
|
|||
.unwrap_or_default();
|
||||
let mtime = file_info.attribute_uint64(gio::FILE_ATTRIBUTE_TIME_MODIFIED);
|
||||
let mut is_desktop = false;
|
||||
let remote = file_info.boolean(gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE);
|
||||
let remote = path.parent().is_none_or(gvfs_dir_is_remote);
|
||||
let is_dir = matches!(file_info.file_type(), gio::FileType::Directory);
|
||||
|
||||
let size_opt = (!is_dir).then_some(file_info.size() as u64);
|
||||
|
|
@ -717,6 +752,7 @@ pub fn item_from_gvfs_info(path: PathBuf, file_info: gio::FileInfo, sizes: IconS
|
|||
mtime,
|
||||
size_opt,
|
||||
children_opt,
|
||||
is_dir,
|
||||
},
|
||||
hidden,
|
||||
image_dimensions: (!remote && mime.type_() == mime::IMAGE)
|
||||
|
|
@ -768,23 +804,7 @@ pub fn item_from_entry(
|
|||
#[cfg(feature = "gvfs")]
|
||||
FsKind::Gvfs => {
|
||||
is_gvfs = true;
|
||||
let file = gio::File::for_path(&path);
|
||||
|
||||
match gio::prelude::FileExt::query_filesystem_info(
|
||||
&file,
|
||||
gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE,
|
||||
gio::Cancellable::NONE,
|
||||
) {
|
||||
Ok(info) => info.boolean(gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE),
|
||||
Err(err) => {
|
||||
log::warn!(
|
||||
"failed to get GIO filesystem info for {}: {}",
|
||||
path.display(),
|
||||
err
|
||||
);
|
||||
true
|
||||
}
|
||||
}
|
||||
path.parent().is_none_or(gvfs_dir_is_remote)
|
||||
}
|
||||
#[cfg(not(feature = "gvfs"))]
|
||||
FsKind::Gvfs => {
|
||||
|
|
@ -1011,7 +1031,6 @@ pub fn scan_path(tab_path: &PathBuf, sizes: IconSizes) -> Vec<Item> {
|
|||
// gio crate expects a comma delimited string
|
||||
let attr_string = [
|
||||
gio::FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME.as_str(),
|
||||
gio::FILE_ATTRIBUTE_FILESYSTEM_REMOTE.as_str(),
|
||||
gio::FILE_ATTRIBUTE_TIME_MODIFIED.as_str(),
|
||||
gio::FILE_ATTRIBUTE_STANDARD_SIZE.as_str(),
|
||||
gio::FILE_ATTRIBUTE_STANDARD_TYPE.as_str(),
|
||||
|
|
@ -1820,6 +1839,8 @@ pub enum Message {
|
|||
HighlightDeactivate(usize),
|
||||
HighlightActivate(usize),
|
||||
DirectorySize(PathBuf, DirSize),
|
||||
#[cfg(feature = "gvfs")]
|
||||
DirectoryChildren(PathBuf, usize),
|
||||
Checksums(PathBuf, ChecksumState),
|
||||
CalculateChecksums(PathBuf),
|
||||
CopyChecksum(String),
|
||||
|
|
@ -1888,6 +1909,7 @@ pub enum ItemMetadata {
|
|||
mtime: u64,
|
||||
size_opt: Option<u64>,
|
||||
children_opt: Option<usize>,
|
||||
is_dir: bool,
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -1902,7 +1924,7 @@ impl ItemMetadata {
|
|||
Self::SimpleDir { .. } => true,
|
||||
Self::SimpleFile { .. } => false,
|
||||
#[cfg(feature = "gvfs")]
|
||||
Self::GvfsPath { children_opt, .. } => children_opt.is_some(),
|
||||
Self::GvfsPath { is_dir, .. } => *is_dir,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -4829,6 +4851,20 @@ impl Tab {
|
|||
}
|
||||
}
|
||||
}
|
||||
#[cfg(feature = "gvfs")]
|
||||
Message::DirectoryChildren(path, children) => {
|
||||
if let Some(ref mut items) = self.items_opt {
|
||||
for item in items.iter_mut() {
|
||||
if item.path_opt() == Some(&path) {
|
||||
if let ItemMetadata::GvfsPath { children_opt, .. } = &mut item.metadata
|
||||
{
|
||||
*children_opt = Some(children);
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Message::Checksums(path, checksum_state) => {
|
||||
let location = Location::Path(path);
|
||||
if let Some(ref mut item) = self.parent_item_opt
|
||||
|
|
@ -4990,11 +5026,15 @@ impl Tab {
|
|||
ItemMetadata::GvfsPath {
|
||||
size_opt,
|
||||
children_opt,
|
||||
is_dir,
|
||||
..
|
||||
} => match children_opt {
|
||||
Some(child_count) => (true, *child_count as u64),
|
||||
None => (false, size_opt.unwrap_or_default()),
|
||||
},
|
||||
} => {
|
||||
if *is_dir {
|
||||
(true, children_opt.unwrap_or_default() as u64)
|
||||
} else {
|
||||
(false, size_opt.unwrap_or_default())
|
||||
}
|
||||
}
|
||||
};
|
||||
let (a_is_entry, a_size) = get_size(a.1);
|
||||
let (b_is_entry, b_size) = get_size(b.1);
|
||||
|
|
@ -6172,17 +6212,21 @@ impl Tab {
|
|||
ItemMetadata::GvfsPath {
|
||||
size_opt,
|
||||
children_opt,
|
||||
is_dir,
|
||||
..
|
||||
} => match children_opt {
|
||||
Some(child_count) => {
|
||||
if *child_count == 1 {
|
||||
format!("{child_count} item")
|
||||
} else {
|
||||
format!("{child_count} items")
|
||||
} => {
|
||||
if *is_dir {
|
||||
// Children are not counted on remote filesystems
|
||||
match children_opt {
|
||||
//TODO: translate
|
||||
Some(1) => "1 item".to_string(),
|
||||
Some(child_count) => format!("{child_count} items"),
|
||||
None => String::new(),
|
||||
}
|
||||
} else {
|
||||
format_size(size_opt.unwrap_or_default())
|
||||
}
|
||||
None => format_size(size_opt.unwrap_or_default()),
|
||||
},
|
||||
}
|
||||
};
|
||||
|
||||
let row = if condensed {
|
||||
|
|
@ -6948,6 +6992,74 @@ impl Tab {
|
|||
Rectangle::new(point, size)
|
||||
};
|
||||
|
||||
// Count the children of visible directories in the background. Doing it while
|
||||
// scanning costs one directory listing per entry, which stalls remote filesystems.
|
||||
#[cfg(feature = "gvfs")]
|
||||
for item in items {
|
||||
let ItemMetadata::GvfsPath {
|
||||
children_opt: None,
|
||||
is_dir: true,
|
||||
..
|
||||
} = &item.metadata
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
// Skip items that are not visible, or have no determined rect
|
||||
match item.rect_opt.get() {
|
||||
Some(rect) if rect.intersects(&visible_rect) => {}
|
||||
_ => continue,
|
||||
}
|
||||
|
||||
let Some(path) = item.path_opt().cloned() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
struct ChildrenWrapper(PathBuf);
|
||||
impl Hash for ChildrenWrapper {
|
||||
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
|
||||
self.0.hash(state);
|
||||
}
|
||||
}
|
||||
|
||||
subscriptions.push(Subscription::run_with(
|
||||
ChildrenWrapper(path),
|
||||
|ChildrenWrapper(path)| {
|
||||
let path = path.clone();
|
||||
stream::channel(
|
||||
1,
|
||||
move |mut output: futures::channel::mpsc::Sender<_>| async move {
|
||||
let message = {
|
||||
let path = path.clone();
|
||||
tokio::task::spawn_blocking(move || {
|
||||
let children = match fs::read_dir(&path) {
|
||||
Ok(entries) => entries.count(),
|
||||
Err(err) => {
|
||||
log::warn!(
|
||||
"failed to read directory {}: {}",
|
||||
path.display(),
|
||||
err
|
||||
);
|
||||
0
|
||||
}
|
||||
};
|
||||
Message::DirectoryChildren(path, children)
|
||||
})
|
||||
.await
|
||||
.unwrap()
|
||||
};
|
||||
|
||||
if let Err(err) = output.send(message).await {
|
||||
log::warn!("failed to send directory children: {err}");
|
||||
}
|
||||
|
||||
std::future::pending().await
|
||||
},
|
||||
)
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
for item in items {
|
||||
if item.thumbnail_opt.is_some() {
|
||||
// Skip items that already have a mime type and thumbnail
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue