Skip to content

Commit c8ee631

Browse files
ChristineThamclaude
andcommitted
Opening a cloud collection: stop paying a round trip per folder
Two findings, both classification rather than algorithm. **A File-Provider folder was being walked as if it were local.** `ResumableTreeWalk` already knows it is latency-bound — a source declares how many listings it can usefully have in flight, `RemoteTreeSource` says six, and the default is one because a real directory listing is a syscall over a warm cache that gains nothing from overlapping. A vault on iCloud Drive, Dropbox or any other Files provider has a *file path*, so it reaches `LocalTreeSource` and inherited that serial default — but every listing there is an XPC round trip into the provider's extension, and a network fetch behind it for a folder not yet enumerated. N folders, N latencies, end to end. That is where the minutes were. `LocalTreeSource` now reports six for a provider-backed root, detected by path (`/Library/Mobile Documents/`, `/Library/CloudStorage/`) rather than by asking the coordinator — the question is asked once per scan and must not itself be a round trip. An ordinary folder still reports one, which matters: routing width-1 through the concurrency window once made the local walk about four times slower, which is why `theWalkIsCompetitiveWithTheEnumeratorOnARealisticVault` exists. A test measures the overlap on a synthetic tree with a provider's latency profile. **The direct-API path listed one folder at a time when it did not have to.** Six in flight is still N/6 round trips for N folders, and every provider can return a whole subtree in paginated batches — Dropbox `list_folder` with `recursive: true`, Graph `/delta`, Drive's folder query, Box recursed server-side. Dropbox's was already implemented and already in use: it is how `changes(since: nil,)` obtains its cursor. The initial open simply never asked for it. `RemoteStore.listRecursively` is that call, defaulting to nil so a provider gains it one at a time and nothing regresses meanwhile. `RecursiveListingCache` fetches once and answers every `children(of:)` from memory. Deliberately a cache rather than a replacement for the walk: building the tree straight from a recursive listing would discard checkpointing, incremental publishing, per-directory fault isolation and resumption, all of which are worth more on a large interrupted sync than the walk's own bookkeeping costs. The walk is untouched; only its expensive step is made free. Measured: twenty folders cost twenty-one listings before and one recursive request after, finding the same hundred files. 472 app tests, 408 editor. Co-Authored-By: Claude <claude-opus-5> <noreply@anthropic.com>
1 parent 4ad090d commit c8ee631

6 files changed

Lines changed: 365 additions & 1 deletion

File tree

HelloNotes/Core/Remote/DropboxStore.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,26 @@ final class DropboxStore: NSObject, RemoteStore, @unchecked Sendable {
8080
return all
8181
}
8282

83+
/// Every entry under `path`, from one recursive `list_folder` plus its
84+
/// continuations — the same request `changes(since: nil,)` already makes to
85+
/// obtain a cursor, asked here for the entries alone.
86+
///
87+
/// A vault of three hundred folders costs three or four requests this way
88+
/// rather than three hundred listings six at a time.
89+
func listRecursively(path: String) async throws -> [RemoteEntry]? {
90+
var data = try await sendAuthed {
91+
Self.listFolderRequest(path: path, token: $0, recursive: true)
92+
}
93+
var page = try Self.parseListFolderPage(data)
94+
var all = page.entries
95+
while page.hasMore, let cursor = page.cursor {
96+
data = try await sendAuthed { Self.listFolderContinueRequest(cursor: cursor, token: $0) }
97+
page = try Self.parseListFolderPage(data)
98+
all += page.entries
99+
}
100+
return all
101+
}
102+
83103
/// Dropbox's delta: a recursive `list_folder` issues a cursor, and
84104
/// `list_folder/continue` returns everything that changed after it —
85105
/// including deletions, which a plain re-list can only infer by absence.

HelloNotes/Core/Remote/RemoteMirror.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,13 @@ final class RemoteMirror {
326326
var updated = manifest
327327
var seen = Set<String>()
328328
var outcome = RemoteSyncOutcome()
329-
let source = RemoteTreeSource(store: store, remoteRoot: remoteRoot, cacheRoot: cacheRoot)
329+
// One recursive listing for the whole tree where the provider has one,
330+
// consulted by every `children(of:)` below. The walk is unchanged — it
331+
// simply stops paying a round trip per folder. See
332+
// `RecursiveListingCache`.
333+
let source = RemoteTreeSource(
334+
store: store, remoteRoot: remoteRoot, cacheRoot: cacheRoot,
335+
prefetch: RecursiveListingCache(store: store, root: remoteRoot))
330336

331337
// Counted once and then kept, rather than recounted per directory.
332338
// `entries.values.count(where:)` walked the *whole* manifest on every

HelloNotes/Core/Remote/RemoteStore.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,23 @@ protocol RemoteStore: AnyObject, Sendable {
6060
/// design rather than polling the tree.
6161
func changes(since cursor: String?, path: String) async throws -> RemoteChangeSet?
6262

63+
/// Every entry under `path`, at every depth, in as few round trips as the
64+
/// provider allows — or `nil` when it has no such call wired up here.
65+
///
66+
/// **This is the difference between one request and one per folder.** The
67+
/// walk is latency-bound: `RemoteTreeSource` overlaps six listings precisely
68+
/// because each is a round trip the app spends idle, and six-at-a-time is
69+
/// still N/6 latencies for N folders. Every provider can return a whole
70+
/// subtree in paginated batches instead — Dropbox `list_folder` with
71+
/// `recursive: true`, Graph `/delta`, Drive `files.list` with a folder
72+
/// query, Box `/folders/:id/items` recursed server-side — turning a few
73+
/// hundred round trips into a few.
74+
///
75+
/// `nil` is not a failure: the caller falls back to walking directory by
76+
/// directory, so a provider gains this one at a time and nothing regresses
77+
/// while it does.
78+
func listRecursively(path: String) async throws -> [RemoteEntry]?
79+
6380
/// A cursor marking "everything up to now", **without** fetching any data.
6481
///
6582
/// Taken at the end of a full sync so the *first* refresh can already use
@@ -73,6 +90,7 @@ protocol RemoteStore: AnyObject, Sendable {
7390
}
7491

7592
extension RemoteStore {
93+
func listRecursively(path: String) async throws -> [RemoteEntry]? { nil }
7694
func changes(since cursor: String?, path: String) async throws -> RemoteChangeSet? { nil }
7795
func latestCursor(path: String) async throws -> String? { nil }
7896
}

HelloNotes/Core/Remote/RemoteTreeSource.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,27 @@ struct RemoteTreeSource: TreeSource {
3838
/// 429 finishes later than one that never asked for it.
3939
var listingConcurrency: Int { 6 }
4040

41+
/// Answers every listing from one recursive fetch, when the provider can do
42+
/// one. Shared by reference so the `struct` stays a value the walk can copy.
43+
var prefetch: RecursiveListingCache? = nil
44+
4145
func children(of directory: String) async throws -> DirectoryListing {
46+
// **One request for the tree, rather than one per folder.**
47+
//
48+
// The walk stays exactly as it is — frontier, checkpoints, progress,
49+
// per-directory fault isolation, resumption — and simply stops paying a
50+
// network latency for each step. Overlapping six listings still leaves
51+
// N/6 round trips for N folders; this leaves a handful for the whole
52+
// tree. A provider without the call returns nil and nothing changes.
53+
if let prefetch, let cached = await prefetch.children(of: remotePath(forRelative: directory)) {
54+
return listing(from: cached)
55+
}
4256
let entries = try await store.list(path: remotePath(forRelative: directory))
57+
return listing(from: entries)
58+
}
59+
60+
/// One directory's entries, as the walk's own shape.
61+
private func listing(from entries: [RemoteEntry]) -> DirectoryListing {
4362
var listing = DirectoryListing()
4463
listing.children.reserveCapacity(entries.count)
4564
for entry in entries {
@@ -81,3 +100,62 @@ struct RemoteTreeSource: TreeSource {
81100
return remotePath(forRelative: relative)
82101
}
83102
}
103+
104+
/// One recursive listing, fetched once and then answered from memory.
105+
///
106+
/// **Why a cache rather than replacing the walk.** The obvious use of a
107+
/// recursive listing is to build the tree from it directly and skip
108+
/// `ResumableTreeWalk` entirely — and that would throw away the checkpointing,
109+
/// the incremental publishing, the per-directory fault isolation and the
110+
/// resumption that make a large, interrupted sync survivable. All of that is
111+
/// worth more than the walk's own bookkeeping costs. So the walk is left intact
112+
/// and only its *expensive* step is made free.
113+
///
114+
/// Failure is not fatal by design: a provider that has no recursive call, or one
115+
/// whose call fails, leaves `entries` nil and every listing falls through to
116+
/// `store.list` exactly as before. The attempt is made once — a provider that
117+
/// refused will refuse again, and retrying per directory would be slower than
118+
/// never having tried.
119+
actor RecursiveListingCache {
120+
private let store: RemoteStore
121+
private let root: String
122+
private var byDirectory: [String: [RemoteEntry]]?
123+
private var attempted = false
124+
125+
init(store: RemoteStore, root: String) {
126+
self.store = store
127+
self.root = root
128+
}
129+
130+
/// The entries directly inside `path`, or nil when there is no prefetch to
131+
/// answer from.
132+
func children(of path: String) async -> [RemoteEntry]? {
133+
if !attempted {
134+
attempted = true
135+
byDirectory = await fetch()
136+
}
137+
return byDirectory?[normalise(path)] ?? (byDirectory == nil ? nil : [])
138+
}
139+
140+
private func fetch() async -> [String: [RemoteEntry]]? {
141+
// `try?` flattens the double optional, so "threw" and "not supported"
142+
// arrive the same way — which is what we want: both mean walk instead.
143+
guard let entries = try? await store.listRecursively(path: root) else { return nil }
144+
// Group by parent. A recursive listing arrives flat and in no
145+
// particular order, and the walk asks for one directory at a time.
146+
var grouped: [String: [RemoteEntry]] = [:]
147+
for entry in entries {
148+
let parent = normalise((entry.path as NSString).deletingLastPathComponent)
149+
grouped[parent, default: []].append(entry)
150+
}
151+
return grouped
152+
}
153+
154+
/// Providers disagree about the trailing slash and about the case of the
155+
/// root; the walk asks with whatever `remotePath(forRelative:)` produced.
156+
private func normalise(_ path: String) -> String {
157+
var p = path
158+
while p.hasSuffix("/") { p.removeLast() }
159+
return p.lowercased()
160+
}
161+
}

HelloNotes/Core/ResumableTreeWalk.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,47 @@ nonisolated enum ResumableTreeWalk {
356356
/// A folder on disk, listed one level at a time.
357357
nonisolated struct LocalTreeSource: TreeSource {
358358
let root: URL
359+
360+
/// How many listings to keep in flight.
361+
///
362+
/// **A folder on a File Provider is not a local folder wearing a path.**
363+
/// The default of 1 is right for a real directory — a listing there is a
364+
/// syscall over a warm cache and overlapping them buys nothing. It is
365+
/// exactly wrong for iCloud Drive or any other Files provider, where
366+
/// `contentsOfDirectory` is an XPC round trip into the provider's extension
367+
/// and, for a folder it has not enumerated yet, a network fetch behind that.
368+
/// Those are latencies with the app doing nothing at all, so a tree of N
369+
/// folders cost N of them laid end to end — which is how adding a large
370+
/// cloud vault came to take minutes.
371+
///
372+
/// The classification was the bug: such a vault reaches
373+
/// `LocalTreeSource` because it *has* a file path, inherited the serial
374+
/// default meant for warm-cache syscalls, and so was walked one directory
375+
/// at a time. `RemoteTreeSource` had already worked this out for the
376+
/// direct-API providers and overlaps six.
377+
///
378+
/// Nothing changes for an ordinary folder, which is the point: the width is
379+
/// 1 unless the root is demonstrably provider-backed.
380+
var listingConcurrency: Int { Self.isProviderBacked(root) ? 6 : 1 }
381+
382+
/// Whether `url` lives behind a file provider rather than on the disk.
383+
///
384+
/// Two shapes, both by path, and deliberately not by asking the provider —
385+
/// the question is asked once per scan and must not itself be a round trip:
386+
///
387+
/// * `…/Library/Mobile Documents/…` — iCloud Drive, including another
388+
/// app's ubiquity container, which is where an Obsidian vault lives.
389+
/// * `…/Library/CloudStorage/…` — every other Files provider on macOS
390+
/// (Dropbox, Google Drive, OneDrive, Box).
391+
///
392+
/// `FileManager.isUbiquitousItem` would answer the first case and not the
393+
/// second, and answers it by asking the coordinator — so it is both
394+
/// narrower and more expensive than looking at the path.
395+
static func isProviderBacked(_ url: URL) -> Bool {
396+
let path = url.path
397+
return path.contains("/Library/Mobile Documents/")
398+
|| path.contains("/Library/CloudStorage/")
399+
}
359400
/// When false, non-Markdown files are dropped during the listing rather than
360401
/// collected and filtered later — the difference between reading and
361402
/// discarding a hundred thousand names, and ignoring them.

0 commit comments

Comments
 (0)