Repository navigation
perf(windows): stream Cloud Files hydration with one range request - #519
Conversation
Opening a cloud-only file makes Windows issue one FETCH_DATA for the whole file. The provider split it into 4 MiB chunks and made a separate ETag-pinned range request per chunk, buffering each response before handing it to CfExecute. Every chunk paid a full server round trip, and servers that must seek to serve a range paid more for each later chunk, so hydration ran far slower than an ordinary download. The aligned span of each request is now fetched with one streamed range response and transferred in 1 MiB chunks as bytes arrive. Cancellation is still checked between chunks. The range transport moves out of DesktopNextcloudServices into DesktopFileRangeTransport, which serves both the existing bounded reads and the new stream with the same If-Match, exact Content-Range and length checks. Hydration planning and the read-handle contract move into WindowsCloudFilesHydration.kt. NextcloudFileRangeSession gains a stream call that falls back to bounded reads on platforms without streaming.
Obiente previewNC Native · Obiente updates this comment as the preview changes. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71e748dd88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CANCEL_FETCH_DATA only set a flag that was checked after the next 1 MiB chunk arrived, so a slow or stalled response kept a cancelled request's connection and provider worker busy until another chunk or a network timeout. Each callback request now has a WindowsCloudRequestCancellation. Its cancel() also closes the read handle the request has open, which cancels the in-flight HTTP call; a request cancelled before it starts never opens a stream. Provider shutdown cancels in-flight hydrations the same way.
…on' into perf/windows-cloud-files-hydration
Outcome
Opening a cloud-only file from the Windows Cloud Files root now downloads it with one streamed range request, so hydration runs at roughly the speed of an ordinary download. Before this change it was much slower: Windows showed "Photos is downloading" for a long time on a single JPEG.
Cause. Measured on a real sync root on this machine: when an application reads a placeholder sequentially, Windows issues one
FETCH_DATAwhose required range is the whole file. The optional range was empty. For example, a 24 MiB file arrived asrequired=0+25165824. The provider split that range into 4 MiB chunks and made a separate request for each one. Every request was ETag-pinned and fully buffered beforeCfExecutereceived any of it. That means:CfExecutenever overlapped.Change.
hydrateWindowsCloudRange(newWindowsCloudFilesHydration.kt) fetches the aligned span of eachFETCH_DATAwith one streamed range. It transfers 1 MiBCfExecutechunks as bytes arrive, checks cancellation between chunks, and fails if any chunk does not match the plan. Hydration planning and the read-handle contract move into the same file.DesktopFileRangeTransport(new, extracted fromDesktopNextcloudServices) serves both the existing boundedreadand the newstream. Both keep the existing guarantees:If-Matchon the pinned ETag, HTTP 206 required, an exactContent-Rangematch, and failure when the body is shorter or longer than the range.NextcloudFileRangeSession.streamis new. Platforms without streaming fall back to one bounded read per chunk, so Android behavior is unchanged.DesktopNextcloudServices.ktshrinks by 46 lines andWindowsCloudFilesProvider.ktby 44. Both size baselines are lowered.Verification
bash tools/check-repository.shpasses (after the fragment received this PR number)changes/unreleased/fragment records the changeDesktop tests on Windows 11 (10.0.26200), JDK 21, all with nothing skipped:
WindowsCloudFilesHydrationTest(new)DesktopFileRangeTransportTest(new, MockWebServer)WindowsCloudFilesProviderTestWindowsCloudFilesDehydrationTest(real sync root, hydrates real placeholders)WindowsCloudFilesStartupTestThe new tests check:
WindowsCloudFilesHydrationTest: a whole-file request makes exactly one backend request and delivers aligned chunks with exact content; an interior request covers only its aligned span; the non-streaming fallback still delivers the same bytes; cancellation stops at a chunk boundary; a short chunk fails before it is transferred.DesktopFileRangeTransportTest: one request carries theRangeandIf-Matchheaders and delivers chunks in order; bounded reads still work; HTTP 412, a mismatchedContent-Range, a truncated body and an oversized body each fail before any chunk is delivered, and each failure is recorded; a closed transport makes no request.Also run:
./gradlew :androidApp:compileDebugKotlinsucceeds, andbash tools/check-kotlin-architecture.shpasses.The FETCH_DATA pattern above came from a temporary probe on a real sync root. The probe is not included in this PR.
Not run:
stream.Compatibility and risk
FETCH_DATArequest with the bytes delivered so far. This is the same as a failure on a later chunk before this change. Windows can retry the read.FETCH_DATA, as before. The provider executor still runs at most 4 at a time.Visual changes
Not applicable.