Skip to content

perf(snapshot): minor memory allocation and usage reduction without cloning#3903

Merged
rtyler merged 2 commits intodelta-io:mainfrom
fvaleye:perf/snapshot-optimization
Oct 28, 2025
Merged

perf(snapshot): minor memory allocation and usage reduction without cloning#3903
rtyler merged 2 commits intodelta-io:mainfrom
fvaleye:perf/snapshot-optimization

Conversation

@fvaleye
Copy link
Copy Markdown
Collaborator

@fvaleye fvaleye commented Oct 28, 2025

Description

Following this PR:

  • use std::mem:take to reduce memory allocation and consumption without cloning in updating EagerSnapshot

Benchmark

From 100 rows to 1k rows, on my local laptop with a simple divan script.

Test case with a small table (a few Parquet files):

  • 5-10 files
  • 100-1,000 rows created only a few files

Results:

  • Fewer allocations: 16-56 allocations eliminated per update() call
  • Less memory: 2-10 KB saved per operation
  • Fewer deallocations: 114-154 fewer deallocations

@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Oct 28, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.74%. Comparing base (a474130) to head (3bff800).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3903      +/-   ##
==========================================
- Coverage   73.76%   73.74%   -0.02%     
==========================================
  Files         151      151              
  Lines       39396    39374      -22     
  Branches    39396    39374      -22     
==========================================
- Hits        29059    29038      -21     
  Misses       9025     9025              
+ Partials     1312     1311       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think github really wants you to remove this import.

hntd187
hntd187 previously approved these changes Oct 28, 2025
@fvaleye fvaleye requested a review from Copilot October 28, 2025 20:28
@fvaleye
Copy link
Copy Markdown
Collaborator Author

fvaleye commented Oct 28, 2025

Thanks @hntd187!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 optimizes the EagerSnapshot::update method by eliminating an unnecessary clone operation when updating snapshot files.

  • Replaced self.files.clone() with std::mem::take(&mut self.files) to avoid cloning the file vector

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…sumption without cloning

Signed-off-by: Florian Valeye <florian.valeye@gmail.com>
…sumption without cloning

Signed-off-by: Florian Valeye <florian.valeye@gmail.com>
@rtyler rtyler enabled auto-merge (rebase) October 28, 2025 22:11
@rtyler rtyler merged commit 8f22045 into delta-io:main Oct 28, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binding/rust Issues for the Rust crate

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants