Skip to content

chore: rm redundant clones - #2325

Merged
LesnyRumcajs merged 1 commit into
masterfrom
rm-redundant-clones
Aug 18, 2026
Merged

chore: rm redundant clones#2325
LesnyRumcajs merged 1 commit into
masterfrom
rm-redundant-clones

Conversation

@LesnyRumcajs

Copy link
Copy Markdown
Contributor

Got rid of some redundant clones here while I was doing the same in Forest.

Copilot AI lite review requested due to automatic review settings August 13, 2026 17:27
@LesnyRumcajs
LesnyRumcajs requested a review from rvagg as a code owner August 13, 2026 17:27
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Aug 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes a few redundant heap allocations/clones across the SDK and FVM implementation, reducing unnecessary copying while preserving behavior.

Changes:

  • Avoid cloning return buffers when constructing IpldBlock responses in the SDK.
  • Serialize pieces directly without allocating an intermediate Vec<PieceInfo>.
  • Remove redundant PathBuf cloning and avoid TokenAmount cloning in gas cost computation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
sdk/src/lib.rs Moves the already-owned return buffer into IpldBlock instead of cloning it.
sdk/src/crypto.rs Serializes the pieces slice directly, avoiding an intermediate allocation.
fvm/src/kernel/default.rs Passes &PathBuf to create_dir_all instead of cloning the path.
fvm/src/executor/default.rs Computes gas cost by borrowing gas_fee_cap rather than cloning it.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

codecov-commenter commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.04%. Comparing base (ef0a993) to head (3266f89).

Files with missing lines Patch % Lines
fvm/src/kernel/default.rs 0.00% 1 Missing ⚠️
sdk/src/crypto.rs 0.00% 1 Missing ⚠️
sdk/src/lib.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2325   +/-   ##
=======================================
  Coverage   77.04%   77.04%           
=======================================
  Files         143      143           
  Lines       14203    14203           
=======================================
  Hits        10943    10943           
  Misses       3260     3260           
Files with missing lines Coverage Δ
fvm/src/executor/default.rs 73.97% <100.00%> (ø)
fvm/src/kernel/default.rs 80.72% <0.00%> (ø)
sdk/src/crypto.rs 0.00% <0.00%> (ø)
sdk/src/lib.rs 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-project-automation github-project-automation Bot moved this from 📌 Triage to ✔️ Approved by reviewer in FilOz Aug 18, 2026
@LesnyRumcajs
LesnyRumcajs merged commit 5fc2684 into master Aug 18, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Aug 18, 2026
@LesnyRumcajs
LesnyRumcajs deleted the rm-redundant-clones branch August 18, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

4 participants