perf: run media sanitizing and file crypto off the main isolate - #703
Conversation
Image sanitizing uses package:image (pure Dart): decode + re-encode of a phone photo takes seconds, and it ran on the UI isolate for every image sent. Whole-file ChaCha20-Poly1305 (pointycastle, up to 25 MB) ran there too for every media send, download and history auto-download, as did the O(n*m) macro-pattern scan over document bytes. - MediaValidationService.validateAndSanitizeImage / ...Light keep their signatures but execute through Isolate.run. - EncryptionService gains encryptToBlobAsync / decryptFromBlobAsync (Isolate.run over the existing sync implementations); the encrypted image/file upload services use them for both directions. - FileValidationService's macro scan runs through Isolate.run. Byte buffers and simple exception objects transfer across the isolate boundary; error propagation is pinned by test.
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03032dcd66
ℹ️ 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".
|
|
||
| // 2. Encrypt with ChaCha20-Poly1305 | ||
| final encryptionResult = EncryptionService.encryptChaCha20Poly1305( | ||
| final encryptedBlob = await EncryptionService.encryptToBlobAsync( |
There was a problem hiding this comment.
This replacement removes the only declaration of encryptionResult, but the result construction below still evaluates _bytesToHex(encryptionResult.nonce). Any build that includes this service therefore fails with an undefined identifier, preventing encrypted file uploads from compiling; derive the nonce from the blob prefix or return it alongside the blob from the async API.
Useful? React with 👍 / 👎.
|
|
||
| // 3. Encrypt with ChaCha20-Poly1305 | ||
| final encryptionResult = EncryptionService.encryptChaCha20Poly1305( | ||
| final encryptedBlob = await EncryptionService.encryptToBlobAsync( |
There was a problem hiding this comment.
This replacement likewise eliminates encryptionResult, while EncryptedImageUploadResult is still initialized with _bytesToHex(encryptionResult.nonce) below. Consequently, targets containing this service cannot compile and encrypted-image uploads are unavailable; extract the nonce from encryptedBlob or expose it from the asynchronous encryption operation.
Useful? React with 👍 / 👎.
…onResult-returning isolate variant
2 participants
Summary
Item 3.5 of the performance plan. Three CPU-heavy media paths ran on the UI isolate:
package:image, pure Dart): decode + re-encode of a 12 MP photo takes 1–5 s, per image sent (media_validation_service.dart).encryption_service.dart).Changes
validateAndSanitizeImage/validateAndSanitizeImageLightkeep their public signatures but execute throughIsolate.run(byte buffers andMediaValidationResult/exceptions transfer across the boundary; propagation pinned by test).EncryptionService.encryptToBlobAsync/decryptFromBlobAsync:Isolate.runover the existing sync implementations (which stay for callers that need them and for the isolate itself). The encrypted image/file upload services now use the async variants in both directions.Isolate.runat both call sites.Left for 3.4 (worker isolate): per-message NIP-44/Schnorr work — batching those into a long-lived worker is the remaining phase-3 item, done after the caches (#700, #701, #702) so the isolate doesn't inherit redundant work.
Test plan
test/services/media_crypto_isolate_test.dart(RED onmain): async blob roundtrip + cross-compat with sync, tampered-blob failure across the isolate, PNG light/heavy sanitize results, garbage-input error propagationfile_messaging_test.dart+ chat suites — greenflutter test— all greenflutter analyze— no new issues🤖 Generated with Claude Code
https://claude.ai/code/session_018fTxqxhpdL5siTgKZqwtur