Skip to content

perf(daemon): skip service binary copy when unchanged#654

Merged
wizzomafizzo merged 3 commits intomainfrom
perf/skip-binary-copy-when-unchanged
Apr 11, 2026
Merged

perf(daemon): skip service binary copy when unchanged#654
wizzomafizzo merged 3 commits intomainfrom
perf/skip-binary-copy-when-unchanged

Conversation

@wizzomafizzo
Copy link
Copy Markdown
Member

@wizzomafizzo wizzomafizzo commented Apr 10, 2026

Summary

  • Compare source and destination binaries before copying in prepareBinary to avoid unnecessary writes on SD card devices
  • Size pre-filter + streaming byte comparison; errors fall through to normal copy

Summary by CodeRabbit

  • New Features

    • Prevents unnecessary binary rewrites by detecting when the destination already matches the source.
  • Tests

    • Added comprehensive tests covering file comparisons (identical, empty, different content/size, missing files) and binary update flows (skips copy when identical; rewrites when content differs).

Compare source and destination binaries before copying in prepareBinary.
Uses a size pre-filter followed by streaming byte comparison to avoid
unnecessary writes on SD card devices where write wear and speed are
concerns.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f745fa0-43de-4f64-a79f-30dd40af3b51

📥 Commits

Reviewing files that changed from the base of the PR and between 80ce7e7 and 04fa34f.

📒 Files selected for processing (1)
  • pkg/service/daemon/daemon_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/service/daemon/daemon_test.go

📝 Walkthrough

Walkthrough

Added a file-comparison helper filesEqual that compares sizes and contents of two files. prepareBinary now calls filesEqual and skips copying when source and destination are identical; it logs debug or warning messages and otherwise retains the original copy flow.

Changes

Cohort / File(s) Summary
Daemon logic & tests
pkg/service/daemon/daemon.go, pkg/service/daemon/daemon_test.go
Added filesEqual(pathA, pathB) (bool, error) to compare files by size then streamed chunk reads; prepareBinary calls it to skip redundant copies (logs on skip or compare failure). Expanded tests cover identical files, empty files, differing content/size, missing source/destination, and copy/no-copy behaviors including mtime verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble bytes beneath the moonlit sky,
I check two files before I copy by,
If every chunk and timestamp sings the same,
I hop away — no needless write to blame,
Contentment found in small efficient tries.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding a performance optimization to skip copying the service binary when it is unchanged.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/skip-binary-copy-when-unchanged

Comment @coderabbitai help to get the list of available commands and usage tips.

@sentry
Copy link
Copy Markdown

sentry bot commented Apr 10, 2026

Codecov Report

❌ Patch coverage is 76.31579% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/service/daemon/daemon.go 76.31% 5 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/service/daemon/daemon_test.go`:
- Around line 190-198: In TestFilesEqual_TargetMissing and
TestFilesEqual_SourceMissing replace hardcoded absolute paths with temp-dir
based paths: use t.TempDir() combined with filepath.Join to create non-existent
paths (e.g., filepath.Join(t.TempDir(), "does-not-exist", "file")) instead of
"/nonexistent/file" and create distinct TempDir()-based paths for both arguments
in filesEqual calls; update the calls to filesEqual in these tests (functions
TestFilesEqual_TargetMissing and TestFilesEqual_SourceMissing) to use those
filepath.Join(t.TempDir(), ...) values to ensure cross-platform compatibility.

In `@pkg/service/daemon/daemon.go`:
- Around line 224-276: Update filesEqual to accept an afero.Fs parameter and
replace all direct os.Stat/os.Open calls with the afero equivalents (use
fs.Stat, afero.Open or afero.ReadFile/Reader helpers) so the function signature
becomes filesEqual(fs afero.Fs, pathA, pathB string) (bool, error); update every
call site that invokes filesEqual in this package to pass afero.NewOsFs() in
production code and inject test filesystems (e.g., afero.NewMemMapFs()) in
tests, ensuring error handling and defer Close semantics remain identical.
🪄 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: 294fb681-501e-4fe6-8db6-449246d8bd82

📥 Commits

Reviewing files that changed from the base of the PR and between 2643e27 and 80ce7e7.

📒 Files selected for processing (2)
  • pkg/service/daemon/daemon.go
  • pkg/service/daemon/daemon_test.go

Comment thread pkg/service/daemon/daemon_test.go Outdated
Comment thread pkg/service/daemon/daemon.go
Use filepath.Join(t.TempDir(), ...) instead of hardcoded absolute
paths like "/nonexistent/file" for cross-platform compatibility.
@wizzomafizzo wizzomafizzo merged commit dd06674 into main Apr 11, 2026
14 checks passed
@wizzomafizzo wizzomafizzo deleted the perf/skip-binary-copy-when-unchanged branch April 11, 2026 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant