PE-9103: Parallelize upload preparation pipeline#2161
Conversation
…ation, plans PE-9103 - parallelize conflict detection: check all files concurrently via Future.wait instead of sequential DB query per file - pre-compute all file lengths in parallel at start of preparation, cache in _fileLengthCache to avoid redundant sequential reads - parallelize cost estimation: turbo balance + AR sizes fetched in parallel, then AR and Turbo cost calculations run concurrently - parallelize upload plan creation: AR and Turbo plans created simultaneously via Future.wait instead of sequentially - remove duplicate wallet mismatch check (called twice in prepareUploadPlanAndCostEstimates) - skip ANT records re-fetch if already populated from initial fire-and-forget fetch - reduce 100ms artificial UI delay to Duration.zero (just yields to event loop for state update) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpload preparation logic now caches file lengths, reduces sequential conflict and guard checks, and parallelizes upload plan mounting and payment estimation. ChangesUpload preparation concurrency
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UploadCubit
participant FileSystem
participant FileRepository
participant UploadPreparer
participant UploadPaymentEvaluator
UploadCubit->>FileSystem: compute selected file lengths concurrently
UploadCubit->>FileRepository: query conflicting names concurrently
UploadCubit->>UploadPreparer: startUploadPreparation
UploadPreparer->>UploadPreparer: mount AR and Turbo plans with Future.wait
UploadPreparer->>UploadPaymentEvaluator: getUploadPaymentInfoForUploadPlans
UploadPaymentEvaluator->>UploadPaymentEvaluator: fetch balance and sizes in parallel
UploadPaymentEvaluator->>UploadPaymentEvaluator: estimate AR and Turbo costs concurrently
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/core/upload/uploader.dart (1)
440-451: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the Turbo estimate instead of recalculating it.
For non-free Turbo uploads,
turboCostFuturecalculates the Turbo cost, then_determineUploadMethod()calls_turboUploadCostCalculator.calculateCost()again. ReuseturboCostEstimateto avoid the duplicate network-bound cost estimate and inconsistent fallback behavior.Proposed refactor
uploadMethod = isFreeUploadPossibleUsingTurbo ? UploadMethod.turbo - : await _determineUploadMethod( - turboBalance.balance, - turboBundleSizes, - _appConfig.allowedDataItemSizeForTurbo, - _isTurboAvailableToUploadAllFiles, - ); + : _isTurboAvailableToUploadAllFiles && + turboBalance.balance >= turboCostEstimate.totalCost + ? UploadMethod.turbo + : UploadMethod.ar;Also applies to: 473-480
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/core/upload/uploader.dart` around lines 440 - 451, Reuse the existing Turbo cost result instead of calling the cost calculator twice: the `turboCostFuture` in `uploader.dart` already produces `turboCostEstimate`, so update `_determineUploadMethod()` (and the related non-free Turbo path in the surrounding upload selection flow) to consume that cached estimate rather than invoking `_turboUploadCostCalculator.calculateCost()` again. Keep the fallback behavior centralized in the `catchError` handling for `turboCostFuture`, and ensure the upload method decision uses the already awaited `turboCostEstimate` value consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/blocs/upload/upload_cubit.dart`:
- Around line 105-107: The file length cache in UploadCubit is using a derived
string key from getIdentifier(), which can collide for different files and
overwrite cached lengths. Update _fileLengthCache and its read/write sites in
UploadCubit, especially around _getTotalSize() and the upload preparation logic,
to key by the file object or IOFile identity instead of a string identifier.
Make sure the cache lookup and population use the same identity-based key so
distinct files cannot share entries.
- Around line 1078-1080: The manifest reload guard in the upload cubit is
checking for a null `_ants` value even though `_ants` is initialized as an empty
list, so the reload path never runs. Update the condition in the upload flow
within the `UploadCubit` manifest handling logic to use the empty-list fallback
by checking `_ants.isEmpty` instead of `_ants == null`, keeping the existing
reload behavior intact when manifest entries are present.
---
Nitpick comments:
In `@lib/core/upload/uploader.dart`:
- Around line 440-451: Reuse the existing Turbo cost result instead of calling
the cost calculator twice: the `turboCostFuture` in `uploader.dart` already
produces `turboCostEstimate`, so update `_determineUploadMethod()` (and the
related non-free Turbo path in the surrounding upload selection flow) to consume
that cached estimate rather than invoking
`_turboUploadCostCalculator.calculateCost()` again. Keep the fallback behavior
centralized in the `catchError` handling for `turboCostFuture`, and ensure the
upload method decision uses the already awaited `turboCostEstimate` value
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 092002cb-3c6c-4110-ad14-4031aa496e51
📒 Files selected for processing (2)
lib/blocs/upload/upload_cubit.dartlib/core/upload/uploader.dart
| /// Pre-computed file lengths cache. Populated in parallel at the start of | ||
| /// upload preparation to avoid redundant sequential file.ioFile.length reads. | ||
| Map<String, int> _fileLengthCache = {}; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid key collisions in the file length cache.
getIdentifier() can collapse distinct files to the same key, especially for empty/blob paths where it returns only ioFile.name. That can overwrite one file’s cached length and make _getTotalSize() report the wrong total. Key the cache by the file object/IOFile identity instead of a derived string.
Proposed fix
- Map<String, int> _fileLengthCache = {};
+ Map<IOFile, int> _fileLengthCache = {};
@@
- size += _fileLengthCache[file.getIdentifier()] ??
+ size += _fileLengthCache[file.ioFile] ??
await file.ioFile.length;
@@
- _files[i].getIdentifier(): lengths[i],
+ _files[i].ioFile: lengths[i],Also applies to: 557-558, 941-945
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/blocs/upload/upload_cubit.dart` around lines 105 - 107, The file length
cache in UploadCubit is using a derived string key from getIdentifier(), which
can collide for different files and overwrite cached lengths. Update
_fileLengthCache and its read/write sites in UploadCubit, especially around
_getTotalSize() and the upload preparation logic, to key by the file object or
IOFile identity instead of a string identifier. Make sure the cache lookup and
population use the same identity-based key so distinct files cannot share
entries.
IOFile.length returns FutureOr<int>, not Future<int>. Future.wait requires Future<T>, so wrap in async closure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mocktail's thenThrow throws synchronously before returning a Future, so .catchError never gets attached. Wrapping in async IIFE with try/catch handles both sync and async throws correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Visit the preview URL for this PR (updated for commit 73d8619): https://ardrive-web--pr2161-pe-9103-upload-prep-yfmy7su9.web.app (expires Sun, 12 Jul 2026 21:24:40 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0 |
1 participant
Summary
Optimizes the upload preparation phase by parallelizing sequential operations that were previously blocking on each other unnecessarily.
Changes:
Future.waitinstead of one sequential query per file. For 100 files: 100 sequential queries → 1 parallel batch.startUploadPreparation()and cached in_fileLengthCache. Eliminates 2-3x redundant sequential reads per file across warning check, plan creation, and total size calculation.Future.waitinstead of sequentially.Duration.zero.Impact
For multi-file uploads (50+ files), preparation time reduced from sequential O(n) DB queries to parallel O(1). Network-bound cost estimation parallelized (saves ~1-2s).
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit