perf: reduce memory usage - #256
Conversation
|
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: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR refactors peer message encoding and stream I/O, changes handshake info hashes from ChangesPeer protocol transport
Torrent and tracker operations
Piece scheduling and live state
Examples and supporting updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PeerTransfer
participant PeerStream
participant PeerMessages
participant TCP
PeerTransfer->>PeerMessages: encode Piece messages
PeerMessages-->>PeerStream: return header and block
PeerStream->>TCP: write vectored frame
TCP-->>PeerStream: deliver bytes
PeerStream-->>PeerTransfer: receive and validate Piece
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@crates/libtortillas/examples/peer_transfer.rs`:
- Around line 25-35: Update the PeerMessages::Piece validation in the
stream.recv loop to verify every byte in block matches the expected 0xa5 value,
in addition to checking the index and BLOCK_LENGTH; retain the existing
unexpected-message error path for any mismatch.
In `@crates/libtortillas/src/protocol/stream.rs`:
- Around line 104-113: Bound the attacker-controlled frame length before
allocation in the stream parsing flow around the length check and
BytesMut::with_capacity. Add a module-level MAX_FRAME_PAYLOAD_LENGTH constant
sized for legitimate piece and extended-metadata messages, reject nonzero
lengths exceeding it, and preserve KeepAlive handling for length == 0.
In `@crates/libtortillas/src/torrent/actor.rs`:
- Around line 766-770: Update the websocket-tracker warning in the tracker
handling flow to log tracker.redacted_endpoint() instead of tracker.uri(),
preserving the existing warning message and match condition.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3dcb162-e43e-4157-a0af-babca148375a
📒 Files selected for processing (11)
crates/libtortillas/examples/peer_transfer.rscrates/libtortillas/src/engine/messages.rscrates/libtortillas/src/lib.rscrates/libtortillas/src/metainfo/file.rscrates/libtortillas/src/pieces/piece_store.rscrates/libtortillas/src/protocol/messages.rscrates/libtortillas/src/protocol/stream.rscrates/libtortillas/src/torrent/actor.rscrates/libtortillas/src/torrent/piece_flow.rscrates/libtortillas/src/torrent/swarm.rscrates/libtortillas/src/tracker/http.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/libtortillas/src/peer/actor.rs (1)
755-761: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd periodic or live-sample publishing for peer transfer rates.
When incoming
PeerMessages::Pieceis handled without publishing,PeerActor::handleonly publishes on state-changing control messages.last_rate_samplealso only gets refreshed in existing publish paths, so the live peer view can carry stalemetrics.transfer.samples,metrics.transfer.rates, and consequently stale download rate/progress between non-piece messages.🤖 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 `@crates/libtortillas/src/peer/actor.rs` around lines 755 - 761, Update PeerActor::handle so incoming PeerMessages::Piece messages periodically publish live transfer samples even when no state-changing publication occurs. Refresh last_rate_sample through the existing rate-sampling path before publishing, ensuring the live PeerView contains current metrics.transfer.samples, metrics.transfer.rates, download rate, and progress. Preserve existing publish behavior and gate the added work consistently with the live feature and publication settings.
🤖 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 `@crates/libtortillas/examples/live.rs`:
- Around line 40-49: Make completion logging in the metrics-handling flow
edge-triggered rather than evaluating complete on every event. Track the
previous completion state or add a one-shot guard so the transition to
remaining_bytes == 0 logs immediately only once; subsequent completed
MetricsChanged events must follow the existing METRICS_LOG_INTERVAL throttle.
In `@crates/libtortillas/src/pieces/piece_scheduler.rs`:
- Around line 144-153: Update the cursor initialization in requests_for_peer to
handle next_piece == piece_count before calling clamp, returning empty request
ranges when all pieces are complete. Preserve normal cursor clamping for
in-progress schedules, including partial final pieces where first_slot may
exceed total_slots.
---
Nitpick comments:
In `@crates/libtortillas/src/peer/actor.rs`:
- Around line 755-761: Update PeerActor::handle so incoming PeerMessages::Piece
messages periodically publish live transfer samples even when no state-changing
publication occurs. Refresh last_rate_sample through the existing rate-sampling
path before publishing, ensuring the live PeerView contains current
metrics.transfer.samples, metrics.transfer.rates, download rate, and progress.
Preserve existing publish behavior and gate the added work consistently with the
live feature and publication settings.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b097dc26-1bbf-4f0c-bcf5-09ac80c34c75
📒 Files selected for processing (8)
crates/libtortillas/examples/live.rscrates/libtortillas/examples/peer_transfer.rscrates/libtortillas/src/peer/actor.rscrates/libtortillas/src/pieces/piece_scheduler.rscrates/libtortillas/src/protocol/stream.rscrates/libtortillas/src/torrent/actor.rscrates/libtortillas/src/tracker/http.rscrates/libtortillas/src/tracker/model.rs
💤 Files with no reviewable changes (1)
- crates/libtortillas/src/tracker/model.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/libtortillas/src/tracker/http.rs
- crates/libtortillas/examples/peer_transfer.rs
- crates/libtortillas/src/torrent/actor.rs
- crates/libtortillas/src/protocol/stream.rs
| let complete = metrics | ||
| .progress | ||
| .remaining_bytes | ||
| .is_some_and(|bytes| bytes.0 == 0); | ||
| let now = Instant::now(); | ||
| if complete | ||
| || last_metrics_log | ||
| .is_none_or(|last| now.duration_since(last) >= METRICS_LOG_INTERVAL) | ||
| { | ||
| last_metrics_log = Some(now); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Make completion logging edge-triggered.
complete remains true after remaining_bytes reaches zero, so every later MetricsChanged event bypasses the interval. If completed torrents continue emitting metrics while seeding, logging becomes unthrottled. Track the false-to-true completion transition or make completion logging a one-shot.
🤖 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 `@crates/libtortillas/examples/live.rs` around lines 40 - 49, Make completion
logging in the metrics-handling flow edge-triggered rather than evaluating
complete on every event. Track the previous completion state or add a one-shot
guard so the transition to remaining_bytes == 0 logs immediately only once;
subsequent completed MetricsChanged events must follow the existing
METRICS_LOG_INTERVAL throttle.
1 participant
No description provided.