Repository navigation
Conversation
Diana calls mutate() on a capture file and then at once on its metadata. On iOS the second mutate() can wait while the first one builds its part files. This adds a way to measure that wait on a device. - harness.chunked20: a chunked kind with Diana's parts (20 MiB, at least 8 MiB). A 2 GiB file gives 103 parts. - "Enqueue latency x7": makes a 2 GiB source one time, then 7 runs. Each run copies the source, calls mutate() on it, then at once calls mutate() on a small JSON request, both on the server's slow mode. It logs nextMs, the time the second mutate() waits, and the median and maximum. - The example README has the steps as iOS step 19. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Since 11.0.0 a chunked mutate() copied every part out of the file and created every part task in one block on the coordinator queue. A mutate() called right after it waited for all of that. For a 2 GiB capture in 20 MiB parts that is 103 part copies, 103 task map writes with fsync and 103 task creations, which can pass the 10 s enqueue timeout on an older iPhone (Sentry DIANA-11QD; out-of-memory crashes DIANA-3QVM, DIANA-3S18). - refill builds the parts in slices of about 32 MiB. Each slice is its own block on the queue, so other calls run between two slices. One continuation per entry. Each slice checks the entry again and computes "pending minus in flight" again, so a part never gets two tasks, and a cancel, pause, replace or expiry between slices stops the build. - A slice creates its tasks suspended, writes their task map entries in one write, then resumes them. Transport.upload no longer resumes. - The background completion handler waits until no slice is queued, in a relaunch reconcile and in urlSessionDidFinishEvents. - os_signpost intervals for enqueue, issue and each slice. - replace() of background-created part tasks goes through refill, so it is sliced too. - README: "starts creating" a task for every part. CHANGELOG: Unreleased entry, and the 11.0.1 line no longer says the task map was batched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fixes from the review of the part slices. - Reconcile resumes every task it adopts, part and simple. A crash between a slice's task map write and its resumes left suspended tasks that held their parts and never sent. resume() on a running task does nothing. - Each slice block runs updateWait after its work, so a row no longer shows a wait while parts of a later slice send. - A background wake stops waiting for queued slices when less than 5 s of background time is left. RNBackgroundUpload passes backgroundTimeRemaining to the coordinator as a closure; grace waits are never cut short. - replace() works one slice at a time and cancels each old task in the slice that replaces it, so a part keeps its old task until its new one exists. It settles an entry that is done, expired or has a short blob, as refill does. - Tests: adopted tasks resume (with and without task map keys, and a simple request), the wait refresh, the wake time cap, activation replace over several slices and while build slices are queued, and the expiry settle on activation. - Docs: the wake trade-off in the README, background-tagged later slices, CHANGELOG lines, the example's before/after commits, and the harness calls resume() and reports when waitGone gives up. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fixes from the second review of the part slices. - A replace slice block with nothing left to replace (each part got a retry task meanwhile) now refills. Before, it queued nothing, so the later parts of a refill plan got no task until a part was accepted. - replaceSlice stops when the app is not active: it drops the rest of the replacement and refills. Before, after a resign, it cancelled background tasks only to make new background tasks. - Reconcile resumes the adopted tasks at its end, after the settles and pause gates, so a task they stopped sends nothing. - The replace doc comment and the CHANGELOG say a part keeps its old task unless its slice's build fails. - Tests for the three fixes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fixes from the pre-merge review of the part slices. - replaceSlice checks again, in every slice, that each old task is live, has sent no body byte and has no response. Before, only replaceBackgroundTasks checked it, when the parts were queued, so a task the daemon started while waiting for its slice could be cancelled mid-upload, with a short window of two PUTs of one part. Such a task is now kept. - The part copy wraps each 1 MiB read in autoreleasepool, so each read buffer is freed before the next. The copy runs inside store.queue.sync on the caller's thread, so a queue autoreleaseFrequency would not cover it. - README and code comments: a mutate() waits for at most one slice per chunked upload that is building, and a next wake is usual but not certain when no slice of an upload could be built. - Tests: a sending old task is kept; a slice that fails partway and the backoff refill; three uploads slicing at once with a small request behind them; a same-id mutate() during a replace chain; a short wake while a grace wait is open. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
dmurphy5
marked this pull request as ready for review
November 10, 2026 20:49
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
dmurphy5
pushed a commit
that referenced
this pull request
Nov 13, 2026
…#80) ## What could go wrong On iOS, a retry's backoff exists only as the begin date of a background task: the library creates the next attempt right away and lets `nsurlsessiond` hold it until the date. If the daemon refuses the app's connection, that task fails immediately with `NSURLErrorDomain -1`, before its begin date and without sending anything. The library counted that as a transient failure and created the next delayed task at once, which failed the same way. That loop ran at 300–600 attempts a second, each writing the entry and opening a new XPC connection. The app sat at about 200% CPU and grew to gigabytes of memory. We hit it on two simulators running diana's CI dev builds, which had no entitlements, so the daemon logged "does not have a bundle ID, rejecting connection" for every session. That build is fixed in openspacelabs/diana#9825. The library should still not spin when the daemon says no. ## What this PR does - **A refused delayed task isn't an attempt.** A delayed task that ends before its begin date, with no response and no bytes sent, never reached the daemon. It emits no event and keeps its ordinal and request id. The app waits out the rest of the delay itself, then issues the attempt again (`advanceAttempt: false`). All three conditions are needed, so a clock change during the wait can't turn a real failure into this. The error code isn't checked, because `-1` also comes with real failures. - **Chunked parts get the same treatment.** A refused part task keeps its begin date and refills once the date passes. Unlike a simple request, the refilled part still takes a new ordinal, as every part task does. - **A floor as a backstop, simple requests only.** A retry never starts within `baseMs` of the previous attempt's start. One that ended before its own start gets no task for `baseMs`. The floor lives in memory only. - **`TaskCompletion` carries `bytesSent`** (from `countOfBytesSent`) so the completion path can tell. Android doesn't have this flaw: it waits out the backoff itself before making the request. ## Review notes - **System cancels change slightly.** If iOS cancels a delayed task before its begin date with nothing sent, the library now waits out the delay under the same ordinal, instead of minting a new attempt with a fresh backoff. It never ran, so that seems right. - **Four existing tests now advance the fake clock before completing a delayed task.** They delivered a server response before the task's begin date, which the real daemon can't do, and that tripped the floor. - **Conflicts with #69 and #72 in `QueueCoordinator+Outcomes.swift`.** It's small: one added line, `attemptStarts[id] = nil`, next to code #69 rewrites. #70 and #65 merge cleanly with this branch and pass the suite (293 and 272 tests). ## How it was tested - `swift test` in `ios/`: 263 tests, 0 failures, 5 new. They cover every task refused for 10 s of fake time (bounded attempts, no tight loop), one refused task keeping its ordinal and X-Request-Id, a task failing after its begin date still counting, a task that sent bytes or got a 503 early still counting, and a refused chunked part. With the fix disabled the new tests fail. - **In diana on an iPhone 15 simulator**, with this branch in `node_modules` and the app built with `CODE_SIGNING_ALLOWED=NO`, so the daemon refuses it: - A stuck upload went from about 5,500 attempts per 30 s to 1 attempt per backoff. - Two new uploads backed off with growing gaps (about 14 s, 32 s, 74 s). - CPU stayed under 2%. - With v11.1.0 the same build spun at 199% CPU. - No JS changed, so `yarn lint:ci`, `typecheck` and `test` weren't run. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Brings in #65, #74, #78 and #80. CHANGELOG.md: kept every entry; master's entries first, then this branch's four (sliced part builds, resume adopted tasks, expired at replace, signposts). #80's iOS changes auto-merged. Checked by reading: in ChunkedCoordinator.partCompleted, a refused part keeps its begin date and schedules refill(id), which now builds in slices and takes the remaining wait from partBeginAt as before. In issue(), attemptStarts is set after task.resume() and before throttle.reset(id). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Brings in #71 (mutate() waits, onWarning). CHANGELOG.md: kept every entry; master's entries first, then this branch's four. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
On iOS, a chunked
mutate()builds a part file and a task for every part. Since 11.0.0 it did all of this in one block on the coordinator queue. For a 2 GiB capture in 20 MiB parts that is 103 part copies (2 GiB read and 2 GiB written), 103 task map writes with fsync, and 103 task creations.Diana calls
mutate()on the capture file and then at once on its metadata. The metadatamutate()waited behind the whole build, and the 10 s enqueue timeout rejected it (Sentry DIANA-11QD, 2.5% of users on every v11 build).Also, the 11.0.1 CHANGELOG said the task map was written once per batch. It was written once per part task.
What changes
refillbuilds the parts in slices of about 32 MiB (two 20 MiB parts). Each slice is its own block on the queue. Other calls run between two slices, so amutate()waits for at most one slice per chunked upload that is building its parts.queuedSlices). Each slice runsrefillagain: it checks the entry again and computes "pending minus in flight" again. So a part never gets two tasks, and a cancel, pause, replace or expiry between two slices stops the build. Each slice block also refreshes the row'snextAttemptAt.Transport.uploadno longer resumes the task. The rule "task map entry before resume" stays.expiresAtnow settlesexpiredat that point instead of getting new tasks (CHANGELOG line).urlSessionDidFinishEvents. It stops waiting for slices when less than 5 s of background time is left (UIApplication.backgroundTimeRemaining, read on main and passed to the coordinator as a closure). Grace waits are never cut short.os_signpostintervals for eachenqueue,issue, part slice and replace slice (subsystemai.openspace.rnbgupload, Points of Interest). Entry ids are logged as public, so they must stay free of user content.mutate(). This is not the v10 lazy window. A slice that runs after the app leaves the foreground tags its tasks as background tasks, and the next activation replaces them, as for any background-created task.No API change.
mutate()resolves at the same point as before.Memory
This PR does not fix the out-of-memory crashes on this path (DIANA-3QVM in
enqueueParts, DIANA-3S18 inTaskMap.write). It changes two things that touch them:autoreleasepool, so each read buffer is freed before the next read. The copy runs insidestore.queue.syncon the caller's thread, so a queue-levelautoreleaseFrequencywould not cover it.Where to look
ios/ChunkedCoordinator.swift:refill,refillNextSlice,replaceandreplaceSlice,runnable, and the task map batch inenqueueParts.ios/QueueCoordinator+Reconcile.swift:whenIdle,releaseIfIdle, the wake time check, andresume()on adopted tasks.ios/RNBackgroundUpload.swift:urlSessionDidFinishEventsholds the handler until the coordinator is idle; thebackgroundTimeRemainingclosure.ios/QueueStore.swift: theautoreleasepoolin the part copy.ios/Transport.swift,ios/TaskMap.swift,ios/Signpost.swift.harness.chunked20kind.What only a device can show
resume(), then runs 7 times: copy a 2 GiB source,mutate()it in 103 parts of 20 MiB, then at oncemutate()a small JSON request, both on the server'sslowmode. It logsnextMs, how long the secondmutate()waits, and the median and maximum. Compare a build of commit 56c51c9 (the harness on top of master's native code) with a build of this PR's tip, on the same device. Target for the tip: mediannextMsunder 500 ms, maximum under 2 s.backgroundTimeRemainingin a background-session relaunch. Look at the value it reports during ahandleEventsForBackgroundURLSessionrelaunch (log it, or break in the debugger). The 5 s cap depends on it: if iOS reports no limit there, the cap never fires.Test plan
cd ios && swift test: 265 tests, 0 failures, 1 skipped (the perf test). Before this PR: 235.RNBGU_PERF=1 swift test --filter ChunkedPerfTests: 200 MiB plan, 10 parts of 20 MiB, real part files, fake sessions. Nextenqueuemedian 17.4 ms with slices, 67.0 ms with the whole plan in one block (on a Mac; fsync dominates there). It does not include nsurlsessiond.yarn lint:ci,yarn typecheck,yarn test(194 tests).yarn test: 5 tests (1 new, for the latency button).RNBackgroundUpload.swifttypechecks against the iOS 15.1 simulator SDK (with stubs for the React symbols).🤖 Generated with Claude Code