mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
perf(fuse): rebuild the hydrated listings in one pass (B167)
The hydrate rebuilt each listed folder's listing with a pass over every entry. With 4,600 listed folders and 37,000 entries that took 1.7 s at every start before the mount came up; one pass grouping the entries by parent takes 3 ms.
This commit is contained in:
1 parent
ffa0fe698d
commit
9bbc994a4f
4 files changed
+89
-15
No files matched your search
+56
-13
@@ -1810,19 +1810,9 @@ impl Core {
|
|||||||
}
|
}
|
||||||
st.hydrate_access();
|
st.hydrate_access();
|
||||||
|
|
||||||
// Pass 3: rebuild child listings for fully-enumerated folders. The root
|
// Pass 3: rebuild child listings for fully-enumerated folders.
|
||||||
// is its own parent (that is what `..` resolves to), so it would match
|
let listings = listed_children(&st.entries, listed_dirs);
|
||||||
// its own filter; a directory listed inside itself makes the kernel fail
|
st.children.extend(listings);
|
||||||
// the lookup with EIO, taking the whole listing down with it.
|
|
||||||
for dir_ino in listed_dirs {
|
|
||||||
let kids: Vec<u64> = st
|
|
||||||
.entries
|
|
||||||
.iter()
|
|
||||||
.filter(|&(&ino, e)| ino != dir_ino && e.parent == dir_ino && !e.node.trashed)
|
|
||||||
.map(|(&ino, _)| ino)
|
|
||||||
.collect();
|
|
||||||
st.children.insert(dir_ino, kids);
|
|
||||||
}
|
|
||||||
info!(nodes = st.entries.len(), "hydrated metadata cache from db");
|
info!(nodes = st.entries.len(), "hydrated metadata cache from db");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -6537,6 +6527,29 @@ fn node_visible(node: &Node, folder_uid: &NodeUid, hidden: &HashSet<NodeUid>) ->
|
|||||||
!node.trashed && node.uid != *folder_uid && !hidden.contains(&node.uid)
|
!node.trashed && node.uid != *folder_uid && !hidden.contains(&node.uid)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The children `entries` hold for each folder in `listed`, gathered in one
|
||||||
|
/// pass over them.
|
||||||
|
///
|
||||||
|
/// The root is its own parent (that is what `..` resolves to), so it would
|
||||||
|
/// count as its own child; a directory listed inside itself makes the kernel
|
||||||
|
/// fail the lookup with EIO, taking the whole listing down with it.
|
||||||
|
///
|
||||||
|
/// A pass per listed folder took 1.7 s on a tree of 37,000 nodes with 4,600 of
|
||||||
|
/// them listed, at every start before the mount came up (`docs/BUGS.md` B167).
|
||||||
|
fn listed_children(entries: &HashMap<u64, Entry>, listed: Vec<u64>) -> HashMap<u64, Vec<u64>> {
|
||||||
|
let mut children: HashMap<u64, Vec<u64>> =
|
||||||
|
listed.into_iter().map(|ino| (ino, Vec::new())).collect();
|
||||||
|
for (&ino, entry) in entries {
|
||||||
|
if ino != entry.parent
|
||||||
|
&& !entry.node.trashed
|
||||||
|
&& let Some(kids) = children.get_mut(&entry.parent)
|
||||||
|
{
|
||||||
|
kids.push(ino);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
children
|
||||||
|
}
|
||||||
|
|
||||||
/// Lay what is queued for a folder over Drive's listing of it, from
|
/// Lay what is queued for a folder over Drive's listing of it, from
|
||||||
/// `Db::queued_children`: a node a queued rename moved away is dropped, and a
|
/// `Db::queued_children`: a node a queued rename moved away is dropped, and a
|
||||||
/// node a queued create, mkdir or rename put there is listed as the DB has it.
|
/// node a queued create, mkdir or rename put there is listed as the DB has it.
|
||||||
@@ -9340,6 +9353,36 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn hydrated_listings_hold_each_folders_live_children() {
|
||||||
|
let entry = |id: &str, parent: u64, is_dir: bool, trashed: bool| {
|
||||||
|
let mut node = node_helper(id, "none", id, is_dir);
|
||||||
|
node.trashed = trashed;
|
||||||
|
crate::state::Entry {
|
||||||
|
uid: node.uid.clone(),
|
||||||
|
parent,
|
||||||
|
node,
|
||||||
|
access: Access::Owner,
|
||||||
|
lookup_count: 1,
|
||||||
|
open_count: 0,
|
||||||
|
unlinked: false,
|
||||||
|
}
|
||||||
|
};
|
||||||
|
let entries: HashMap<u64, crate::state::Entry> = [
|
||||||
|
(1, entry("root", 1, true, false)),
|
||||||
|
(2, entry("a", 1, true, false)),
|
||||||
|
(3, entry("b", 1, true, false)),
|
||||||
|
(4, entry("a1", 2, false, false)),
|
||||||
|
(5, entry("a2", 2, false, true)),
|
||||||
|
(6, entry("b1", 3, false, false)),
|
||||||
|
]
|
||||||
|
.into_iter()
|
||||||
|
.collect();
|
||||||
|
let mut listings = super::listed_children(&entries, vec![1, 2]);
|
||||||
|
listings.values_mut().for_each(|kids| kids.sort());
|
||||||
|
assert_eq!(listings, HashMap::from([(1, vec![2, 3]), (2, vec![4])]));
|
||||||
|
}
|
||||||
|
|
||||||
fn node_helper(id: &str, parent: &str, name: &str, is_dir: bool) -> Node {
|
fn node_helper(id: &str, parent: &str, name: &str, is_dir: bool) -> Node {
|
||||||
let uid = NodeUid::new(VolumeId::from("vol"), LinkId::from(id));
|
let uid = NodeUid::new(VolumeId::from("vol"), LinkId::from(id));
|
||||||
let parent_uid = if parent == "none" {
|
let parent_uid = if parent == "none" {
|
||||||
|
|||||||
@@ -12,6 +12,32 @@ Conventions:
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## B167 — The mount takes seconds to come up on a large Drive
|
||||||
|
|
||||||
|
**Status:** Fixed.
|
||||||
|
**Found:** 2026-10-04, looking into the cleanup of the account run on the working tree after
|
||||||
|
B165, which found no control socket after the last daemon restart. Every start in that run
|
||||||
|
took 2 to 3 s from "mounting" to "hydrated metadata cache from db" (37,000 nodes), the one
|
||||||
|
before the cleanup 8.8 s. That was not the cleanup's failure: the socket is bound before the
|
||||||
|
hydrate, and a request made during it waits. The cleanup ran some 1.5 s after the restart,
|
||||||
|
before the new daemon got as far as binding it. `pdfs status` reports a daemon that is not
|
||||||
|
listening yet as not running, and the run's queue wait took that for an empty queue.
|
||||||
|
|
||||||
|
**Where:** `hydrate` in `crates/pdfs-fuse/src/lib.rs`.
|
||||||
|
|
||||||
|
**Cause.** The hydrate rebuilt the listing of every listed folder with a pass over all
|
||||||
|
entries: 4,600 listed folders times 37,000 entries. That took 1.7 s on an idle machine
|
||||||
|
(measured on a tree of that shape with entries of the real size), before the mount came up.
|
||||||
|
|
||||||
|
**Fix.** The listings are gathered in one pass over the entries (`listed_children`), which
|
||||||
|
takes 3 ms on the same tree.
|
||||||
|
|
||||||
|
**Test:** `hydrated_listings_hold_each_folders_live_children` in `lib.rs`: each listed folder
|
||||||
|
gets its live children, the root is not listed inside itself, and an unlisted folder gets no
|
||||||
|
listing.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## B166 — A folder drops or empties a file whose create lands while it is listed
|
## B166 — A folder drops or empties a file whose create lands while it is listed
|
||||||
|
|
||||||
**Status:** Fixed.
|
**Status:** Fixed.
|
||||||
|
|||||||
@@ -62,6 +62,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
|||||||
- **A folder no longer drops or empties a new file while others are uploaded.** A file whose
|
- **A folder no longer drops or empties a new file while others are uploaded.** A file whose
|
||||||
upload landed while the folder was being listed was missing from that listing, or listed as
|
upload landed while the folder was being listed was missing from that listing, or listed as
|
||||||
empty (B166).
|
empty (B166).
|
||||||
|
- **The mount comes up faster on a large Drive.** Rebuilding the folder listings from the cache
|
||||||
|
took close to 2 s with 37,000 files at every start (B167).
|
||||||
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
||||||
the old name, and a second rename no longer failed with "No such file or directory" (B152).
|
the old name, and a second rename no longer failed with "No such file or directory" (B152).
|
||||||
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
|||||||
- **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0).
|
- **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0).
|
||||||
- **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded
|
- **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded
|
||||||
locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c).
|
locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c).
|
||||||
- Fixes from the simulation and account runs since then: B127 and B141 to B166.
|
- Fixes from the simulation and account runs since then: B127 and B141 to B167.
|
||||||
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
||||||
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`.
|
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`.
|
||||||
|
|
||||||
@@ -104,7 +104,10 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
|||||||
it (B165, bounded). The next run on the working tree (2026-10-04: 259 passed, 4
|
it (B165, bounded). The next run on the working tree (2026-10-04: 259 passed, 4
|
||||||
failed) found a wide folder dropping files whose create landed while it was listed
|
failed) found a wide folder dropping files whose create landed while it was listed
|
||||||
(B166, fixed), or listing one empty: the new file that read back empty again
|
(B166, fixed), or listing one empty: the new file that read back empty again
|
||||||
(B166 too). Still open from it: a cleanup that found no control socket.
|
(B166 too). Its cleanup found no control socket because it ran 1.5 s after the
|
||||||
|
last restart, before the new daemon had bound it: `pdfs status` reports that as not
|
||||||
|
running, which the run's queue wait takes for an empty queue (the script's side).
|
||||||
|
Looking into it found the start spending 1.7 s rebuilding listings (B167, fixed).
|
||||||
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
||||||
|
|
||||||
## After 3.0.0
|
## After 3.0.0
|
||||||
|
|||||||
Reference in new issue
Block a user