Skip to content

Commit 9bd1f41

Browse files
adibhannaadibhanna
authored
Merge pull request #133 from ZenNotes/perf/cloud-pull-bridge-calls
Cloud pulls make 10 bridge calls per file instead of 16
2 parents 25c7951 + 56823ba commit 9bd1f41

7 files changed

Lines changed: 459 additions & 13 deletions

‎docs/cloud-pull-bridge-calls-2026-10-26.md‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# Cloud pull: fewer bridge calls per file, 26 October 2026
2+
3+
Branch `perf/cloud-pull-bridge-calls`, cut from `origin/main` at 3609c83
4+
(1.1.46). Changes: `src/bridge/cloud-sync-repository.ts`,
5+
`src/bridge/native-fs.ts`, `src/bridge/mobile-cloud-sync.ts`, and tests. No
6+
native code, no Capacitor config and no app-core change.
7+
8+
## Why
9+
10+
A Discord report (yeminn) had phones sitting in "Syncing..." through a long
11+
first Cloud sync. Measured on an API 37 emulator, a pull costs the same per
12+
file at 600, 2,500 and 5,000 notes, and that cost is almost all Capacitor
13+
bridge round trips: about 16 per pulled file at about 1 ms each, 10.3 s of a
14+
10.7 s 600-note sync. So the lever for big first syncs is calls per file.
15+
16+
## What changed
17+
18+
Five calls per pulled file were redundant. Each is gone only where it could
19+
not have guarded a file.
20+
21+
| Call | Where | Why it cannot lose a file |
22+
|---|---|---|
23+
| `deleteFile(backup)` on every write | `NativeCloudSyncContent.write` | A backup exists only when an original was moved aside (`before`). A new file never makes one, so the delete always failed into its `.catch`. Replacing a file still deletes its backup. |
24+
| `stat(temporary)` after publishing | same | A rename that resolved moved the temporary onto the path. Capacitor's local rename either `renameTo`s or copies and then deletes the source, rejecting if that delete fails; SAF moves the document. Every path that did not publish still checks and removes the temporary. |
25+
| `readdir(.zennotes/sync)` on every write | `assertReady` | Recovery files are now relisted only when one may have appeared: after a write that failed, after a backup that would not delete, and after any staged publish (which keeps its own backups). The first write, every scan and every staged download still list first. A completed write that removed its backup leaves nothing to find. |
26+
| `Filesystem.getUri` before every `inspect` (local tier) | `NativeFs.readForSync` | `getUri` names a file by the root's URI plus the path with each segment escaped exactly as `encodeURIComponent` does. The root is resolved once at open, and the shortcut is used only for the same `Directory` it was resolved in; otherwise, and before open, it asks `getUri` as before. Checked on the emulator for 13 awkward names: identical strings and identical bytes. SAF vaults still stat for their document URI. |
27+
| `mkdir(parent)` before every rename | `hostVault` in `mobile-cloud-sync.ts` | Each sync operation remembers the folders it has made. A rename into a folder that went missing fails before it changes anything (Capacitor checks the parent first; SAF makes it), and the cache forgets that folder so the next rename makes it again. The next sync starts with an empty cache. |
28+
29+
Kept as they were: the pre-publish `stat(path)` (a new file must still be
30+
absent), both verifies (the temporary before publishing, the path after),
31+
the original's verify before it moves aside, and every rollback.
32+
33+
One stricter behavior: a staged publish (large or referenced files) whose
34+
rollback copy would not delete now stops the next write in the same run
35+
with "Cloud sync needs recovery". Before, the run went on and the next sync's
36+
scan stopped at the same file, so the user lands in the same place one step
37+
sooner, the same way an inline write's leftover backup already behaved.
38+
39+
## Results
40+
41+
API 37 emulator (AVD `yeminn-conflict`, `-gpu host -cores 2 -memory 2048`),
42+
fake Cloud fixture on the Mac, small inline notes in four folders. Fresh app
43+
each run (`pm clear`, onboard, sign in, link), bridge calls counted by
44+
wrapping `Capacitor.nativePromise`. Before and after APKs were built from the
45+
same worktree, differing only in the three source files, and run
46+
alternately. An earlier after build, which joined the root and the path with
47+
a doubled slash (same bytes, same call count), ran 600 notes in 6.84 s and is
48+
left out of the averages.
49+
50+
| Run | Before | After | Change |
51+
|---|---|---|---|
52+
| First sync, 600 notes | 10.13 s, 59.3 notes/s (3 runs) | 6.65 s, 90.4 notes/s (2 runs) | 34% less time |
53+
| Change feed, 1,000 notes added (2 runs each) | 16.44 s, 60.8 notes/s | 11.12 s, 90.0 notes/s | 32% less time |
54+
| First sync, 2,500 notes | 40.70 s, 61.5 notes/s (2 runs) | 28.68 s, 87.2 notes/s (1 run) | 30% less time |
55+
56+
Bridge calls per pulled file, first sync of 600:
57+
58+
| Call | Before | After |
59+
|---|---|---|
60+
| `Filesystem.stat` | 5.02 | 4.02 |
61+
| `ZenDirectUpload.inspect` | 3.00 | 3.00 |
62+
| `Filesystem.getUri` | 2.00 | 0 |
63+
| `Filesystem.readdir` | 1.05 | 0.05 |
64+
| `Filesystem.readFile` | 1.01 | 1.01 |
65+
| `Filesystem.writeFile` | 1.00 | 1.00 |
66+
| `Filesystem.mkdir` | 1.00 | 0.01 |
67+
| `Filesystem.rename` | 1.00 | 1.00 |
68+
| `Filesystem.deleteFile` | 1.00 | 0 |
69+
| **Total** | **16.11** | **10.12** |
70+
71+
The change feed went from 16.5 to 10.5 calls per file, and 2,500 notes from
72+
16.03 to 10.03.
73+
74+
Same results: the vault's files hashed identically (SHA-256 of every file
75+
outside `.zennotes`) between before and after for every pair: 602 files at
76+
600, 1,602 after the change feed, 2,502 at 2,500. `.zennotes/sync` was empty
77+
after every run. Resolving the first-sync conflict with Cloud's version (a
78+
staged publish over an existing note) landed the PC's text and left no
79+
rollback file.
80+
81+
## What is left per file
82+
83+
About 10 calls: `stat` 4 (core's existence check, the write's own, the
84+
pre-publish check, the post-sync scan's stability check), `inspect` 3 (both
85+
verifies, and the scan reading the new file), and one each of `writeFile`,
86+
`rename` and `readFile` (the app's rescan after sync). The core's check and
87+
the write's own `stat` look alike but sit on either side of the core
88+
boundary; merging them is a core API change. Seeding the scan cache from the
89+
write would need its own before and after `stat` to stay trustworthy, which
90+
costs what it saves.
91+
92+
## Tests
93+
94+
`npm test`: 322 pass (312 before, 10 new). `npm run typecheck` and
95+
`npm run boundaries:check` clean.
96+
97+
- `cloud-sync-repository.test.ts`, "bridge calls per pulled file": the exact
98+
call list for a new file (one recovery listing, no backup delete, no
99+
temporary check); a replaced original is still verified, moved aside and its
100+
rollback copy deleted; a rollback copy that will not delete stops the next
101+
write; a failed write lists recovery files again; a staged publish that kept
102+
its rollback copy stops the next write.
103+
- `native-fs-sync-uri.test.ts`: local file URIs come from the root resolved at
104+
open and match Android's `getUri` encoding for awkward names, with or without
105+
the folder slash; before open, or after the root moved, it asks `getUri`;
106+
a known URI and the SAF document lookup are unchanged.
107+
- `mobile-cloud-sync.integration.test.ts`: each folder is made once per sync,
108+
and again on the next sync.
109+
110+
Against the old code, the call-count tests fail (as they should) and the
111+
safety tests pass.
112+
113+
## How to test locally
114+
115+
1. `npm test` and `npm run typecheck` in this checkout.
116+
2. On a device or emulator with a debug build, open the WebView over CDP
117+
(`adb forward tcp:9333 localabstract:webview_devtools_remote_$(adb shell pidof md.zennotes)`)
118+
and count calls during a sync:
119+
120+
```js
121+
const cap = window.Capacitor, stats = {}
122+
cap.__orig ??= cap.nativePromise
123+
cap.nativePromise = function (plugin, method, options) {
124+
const s = (stats[`${plugin}.${method}`] ??= { n: 0 })
125+
s.n++
126+
return cap.__orig.call(this, plugin, method, options)
127+
}
128+
const summary = await window.zen.syncCloudVault()
129+
Object.entries(stats).map(([k, v]) => `${k} ${(v.n / summary.pulled).toFixed(2)}`)
130+
```
131+
132+
A sync that pulls new notes should show no `getUri` or `deleteFile` and
133+
about 10 calls per file in total.
134+
3. Pull a note into a new folder and edit a pulled note on another device,
135+
then sync: both land, and `.zennotes/sync` holds nothing but an empty
136+
`downloads` folder afterwards.

‎src/bridge/cloud-sync-repository.test.ts‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,3 +633,131 @@ describe('cached scan with the pinned conflict coordinator', () => {
633633
assert.deepEqual(h.reads, ['renamed.md'])
634634
})
635635
})
636+
637+
describe('bridge calls per pulled file', () => {
638+
const ID = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/g
639+
/** The repository over call-logging copies of the harness's fs and native files. */
640+
function logged(h: ReturnType<typeof harness>, extra: Record<string, unknown> = {}) {
641+
const calls: string[] = []
642+
const log = <T extends Record<string, unknown>>(target: T) => Object.fromEntries(Object.entries(target).map(
643+
([name, value]) => [name, typeof value !== 'function' ? value : (...args: unknown[]) => {
644+
calls.push([name, ...args.filter((arg) => typeof arg === 'string')].join(' ').replace(ID, '<id>'))
645+
return value(...args)
646+
}]
647+
)) as T
648+
const repository = new CachedCloudSyncRepository(log(h.fs), log({ ...h.native, ...extra }), h.store)
649+
return { calls, repository }
650+
}
651+
const upsert = (sequence: number, path: string, text: string) => ({
652+
sequence, item_id: `item-${sequence}`, revision: 1, type: 'upsert' as const, path, previous_path: null, content: content(text)
653+
})
654+
const tracked = (path: string, text: string) => ({
655+
item_id: 'item-0', path, kind: 'text' as const, revision: 1,
656+
sha256: content(text).sha256, byte_length: Buffer.byteLength(text), media_type: 'text/markdown'
657+
})
658+
659+
it('lists recovery files once and skips backup and temporary cleanup for new files', async () => {
660+
const h = harness({})
661+
const { calls, repository } = logged(h)
662+
await repository.apply(upsert(1, 'a.md', 'A'), undefined)
663+
const first = calls.splice(0)
664+
await repository.apply(upsert(2, 'folder/b.md', 'B'), undefined)
665+
await repository.apply(upsert(3, 'folder/c.md', 'C'), undefined)
666+
667+
assert.deepEqual(first, [
668+
'stat a.md',
669+
'readdirStrict .zennotes/sync',
670+
'stat a.md',
671+
'writeText .zennotes/sync/write-<id>.md A',
672+
'readForSync .zennotes/sync/write-<id>.md',
673+
'stat a.md',
674+
'rename .zennotes/sync/write-<id>.md a.md',
675+
'readForSync a.md'
676+
])
677+
assert.equal(calls.filter((call) => call.startsWith('readdirStrict')).length, 0)
678+
assert.equal(calls.filter((call) => call.startsWith('deleteFile')).length, 0)
679+
assert.equal(calls.filter((call) => call.startsWith('stat .zennotes')).length, 0)
680+
assert.deepEqual([...h.files.keys()].sort(), ['a.md', 'folder/b.md', 'folder/c.md'])
681+
assert.equal(h.files.get('folder/c.md')?.bytes.toString(), 'C')
682+
})
683+
684+
it('still verifies a replaced original and removes its rollback copy', async () => {
685+
const h = harness({ 'note.md': 'Old' })
686+
const { calls, repository } = logged(h)
687+
await repository.apply({ ...upsert(2, 'note.md', 'New'), item_id: 'item-0', revision: 2 }, tracked('note.md', 'Old'))
688+
assert.equal(h.files.get('note.md')?.bytes.toString(), 'New')
689+
assert.deepEqual([...h.files.keys()], ['note.md'])
690+
assert.deepEqual(calls.filter((call) => /^(rename|deleteFile)/.test(call)), [
691+
'rename note.md .zennotes/sync/rollback-<id>.md',
692+
'rename .zennotes/sync/write-<id>.md note.md',
693+
'deleteFile .zennotes/sync/rollback-<id>.md'
694+
])
695+
// The original is verified before it moves aside, the new bytes after they land.
696+
const aside = calls.indexOf('rename note.md .zennotes/sync/rollback-<id>.md')
697+
const landed = calls.indexOf('rename .zennotes/sync/write-<id>.md note.md')
698+
assert.equal(calls[aside - 1], 'readForSync note.md')
699+
assert.equal(calls[landed + 1], 'readForSync note.md')
700+
})
701+
702+
it('stops the next write when a rollback copy would not delete', async () => {
703+
const h = harness({ 'note.md': 'Old' })
704+
const deleteFile = h.fs.deleteFile
705+
h.fs.deleteFile = async (path: string) => {
706+
if (path.includes('/rollback-')) throw new Error('Delete refused')
707+
await deleteFile(path)
708+
}
709+
const repository = new CachedCloudSyncRepository(h.fs, h.native, h.store)
710+
await repository.apply({ ...upsert(2, 'note.md', 'New'), item_id: 'item-0', revision: 2 }, tracked('note.md', 'Old'))
711+
const rollback = [...h.files.keys()].find((path) => path.includes('/rollback-'))
712+
assert.equal(h.files.get(rollback!)?.bytes.toString(), 'Old')
713+
await assert.rejects(repository.apply(upsert(3, 'other.md', 'Other'), undefined), /needs recovery/)
714+
assert.equal(h.files.has('other.md'), false)
715+
})
716+
717+
it('lists recovery files again after a failed write', async () => {
718+
const h = harness({})
719+
const { calls, repository } = logged(h)
720+
h.setWriteHook((path) => {
721+
h.setWriteHook(undefined)
722+
throw new Error(`Native write failed for ${path}`)
723+
})
724+
await assert.rejects(repository.apply(upsert(1, 'a.md', 'A'), undefined), /Native write failed/)
725+
await repository.apply(upsert(2, 'b.md', 'B'), undefined)
726+
assert.equal(calls.filter((call) => call === 'readdirStrict .zennotes/sync').length, 2)
727+
assert.deepEqual([...h.files.keys()], ['b.md'])
728+
})
729+
730+
it('stops the next write when a staged publish kept its rollback copy', async () => {
731+
const h = harness({ 'note.md': 'Old' })
732+
const cloud = Buffer.from('Cloud version')
733+
const ref = {
734+
item_id: 'item-0', revision: 2, encoding: 'utf8', sha256: createHash('sha256').update(cloud).digest('hex'),
735+
byte_length: cloud.length, media_type: 'text/markdown'
736+
}
737+
const { repository } = logged(h, {
738+
statVerified: async (path: string) => (await h.fs.stat(path)),
739+
download: async (options: { to: string }) => { h.put(options.to, cloud) },
740+
rename: h.fs.rename,
741+
deleteFile: async (path: string) => {
742+
if (path.includes('/rollback-')) throw new Error('Delete refused')
743+
await h.fs.deleteFile(path)
744+
},
745+
mkdir: async () => {},
746+
readText: async (path: string) => h.files.get(path)!.bytes.toString()
747+
})
748+
const file = await repository.stageCloudContent!({
749+
reference: ref, previewLimitBytes: 262_144, allowInsecureLoopback: false,
750+
getInstruction: async () => ({
751+
item_id: ref.item_id, revision: ref.revision,
752+
content: { encoding: ref.encoding, sha256: ref.sha256, byte_length: ref.byte_length, media_type: ref.media_type },
753+
download: { url: 'https://objects.example.test/item-0/2', method: 'GET', headers: {},
754+
expires_at: new Date(Date.now() + 300_000).toISOString() }
755+
})
756+
})
757+
await repository.applyStagedCloudContent!({ sequence: 2, item_id: 'item-0', type: 'upsert', path: 'note.md',
758+
previous_path: null, revision: 2, content_ref: ref }, tracked('note.md', 'Old'), file)
759+
assert.equal(h.files.get('note.md')?.bytes.toString(), 'Cloud version')
760+
await assert.rejects(repository.apply(upsert(3, 'other.md', 'Other'), undefined), /needs recovery/)
761+
assert.equal(h.files.has('other.md'), false)
762+
})
763+
})

‎src/bridge/cloud-sync-repository.ts‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,14 @@ export class CachedCloudSyncRepository extends PortableCloudSyncRepository {
8181
await contentFiles.assertReady(true)
8282
return staging.stage(source)
8383
},
84-
applyStagedCloudContent: (...args: Parameters<NativeCloudStaging['apply']>) => staging.apply(...args),
85-
resolveStagedCloudConflict: (input: Parameters<NativeCloudStaging['resolve']>[0]) => staging.resolve(input)
84+
// A staged publish keeps its own rollback copy, so the next read or
85+
// write looks for one again.
86+
applyStagedCloudContent: async (...args: Parameters<NativeCloudStaging['apply']>) => {
87+
try { return await staging.apply(...args) } finally { contentFiles.recheckRecovery() }
88+
},
89+
resolveStagedCloudConflict: async (input: Parameters<NativeCloudStaging['resolve']>[0]) => {
90+
try { return await staging.resolve(input) } finally { contentFiles.recheckRecovery() }
91+
}
8692
} : {}),
8793
readdir: (path: string) => fs.readdir(path),
8894
stat: (path: string) => fs.stat(path),
@@ -207,6 +213,11 @@ class NativeCloudSyncContent {
207213
this.recoveryChecked = true
208214
}
209215

216+
/** Something may have left a rollback file since the last listing. */
217+
recheckRecovery(): void {
218+
this.recoveryChecked = false
219+
}
220+
210221
async read(path: string, uri?: string): Promise<CloudSyncLocalItem> {
211222
await this.assertReady()
212223
const file = await this.native.readForSync(path, TEXT_EXTENSIONS.has(extension(path)), uri)
@@ -270,6 +281,7 @@ class NativeCloudSyncContent {
270281
const temporary = `.zennotes/sync/write-${randomUUID()}${extension(path)}`
271282
const backup = `.zennotes/sync/rollback-${randomUUID()}${extension(path)}`
272283
let publishing = false
284+
let published = false
273285
let completed = false
274286
try {
275287
const source = this.sources.get(content)
@@ -290,6 +302,7 @@ class NativeCloudSyncContent {
290302
}
291303
publishing = true
292304
await this.fs.rename(temporary, path)
305+
published = true
293306
await this.verify(path, content)
294307
completed = true
295308
} catch (error) {
@@ -307,9 +320,13 @@ class NativeCloudSyncContent {
307320
}
308321
throw error
309322
} finally {
310-
this.recoveryChecked = false
311-
if (await this.fs.stat(temporary) === 'file') await this.fs.deleteFile(temporary).catch(() => {})
312-
if (completed) await this.fs.deleteFile(backup).catch(() => {})
323+
// A write that completed and removed its backup leaves no rollback
324+
// file, so the listing it started from still holds.
325+
if (!completed) this.recoveryChecked = false
326+
// A rename that resolved moved the temporary onto the path.
327+
if (!published && await this.fs.stat(temporary) === 'file') await this.fs.deleteFile(temporary).catch(() => {})
328+
// There is a backup only when an original was moved aside.
329+
if (completed && before) await this.fs.deleteFile(backup).catch(() => { this.recoveryChecked = false })
313330
}
314331
}
315332
}

‎src/bridge/mobile-cloud-sync.integration.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,24 @@ describe('mobile Cloud adapter wiring', () => {
225225
})
226226
}
227227

228+
it('makes each folder once per sync however many pulled files land in it', async () => {
229+
const h = await fixture({})
230+
for (const path of ['notes/a.md', 'notes/b.md', 'notes/deeper/c.md', 'notes/d.md', 'top.md']) {
231+
h.remoteText(path, `Content of ${path}`)
232+
}
233+
await h.sync()
234+
assert.deepEqual([...h.files.keys()].sort(), ['notes/a.md', 'notes/b.md', 'notes/d.md', 'notes/deeper/c.md', 'top.md'])
235+
assert.deepEqual(h.createdDirectories, ['notes', 'notes/deeper'])
236+
// A folder removed between syncs comes back: each sync asks again. The
237+
// edit's original moves aside into .zennotes/sync before the new bytes land.
238+
h.createdDirectories.length = 0
239+
h.remoteText('notes/e.md', 'Fifth note')
240+
h.remoteText('notes/b.md', 'Edited on desktop')
241+
await h.sync()
242+
assert.deepEqual(h.createdDirectories, ['notes', '.zennotes/sync'])
243+
assert.equal(h.files.get('notes/b.md')?.bytes.toString(), 'Edited on desktop')
244+
})
245+
228246
it('detects remote changes from a one-item metadata manifest without scanning local files', async () => {
229247
const h = await fixture()
230248
assert.equal(await h.api.hasMobileCloudVaultChanges(h.vault), true)

‎src/bridge/mobile-cloud-sync.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,9 @@ export async function restoreMobileCloudBackupNote(
341341
}
342342

343343
function hostVault(vault: MobileVault, cacheScan = false): CloudSyncHostVault {
344+
// Folders this operation has made, so a pull asks for each one once rather
345+
// than before every file it renames into place.
346+
const folders = new Set<string>()
344347
const fs: PortableCloudSyncFileSystem = {
345348
// Sync must distinguish an unreadable provider from an empty vault.
346349
readdir: async (directory) =>
@@ -356,8 +359,18 @@ function hostVault(vault: MobileVault, cacheScan = false): CloudSyncHostVault {
356359
deleteFile: (path) => vault.fs.deleteFile(path),
357360
rename: async (from, to) => {
358361
const parent = to.includes('/') ? to.slice(0, to.lastIndexOf('/')) : ''
359-
if (parent) await vault.fs.mkdir(parent)
360-
await vault.fs.rename(from, to)
362+
if (parent && !folders.has(parent)) {
363+
await vault.fs.mkdir(parent)
364+
folders.add(parent)
365+
}
366+
try {
367+
await vault.fs.rename(from, to)
368+
} catch (error) {
369+
// The folder may have gone since: a rename into a missing folder
370+
// fails before it changes anything, and the next one makes it again.
371+
folders.delete(parent)
372+
throw error
373+
}
361374
}
362375
}
363376

0 commit comments

Comments
 (0)