Skip to content

Conversation

@gauravsaini
Copy link

@gauravsaini gauravsaini commented May 17, 2025

Related GitHub Issue

Closes #3391
Closes #3348
Closes #3080

Description

Garbage collection and perf improvements
Upgrade to git 2.49.0 for additional benefits

Test Procedure

Unit tests added
Sanity test on local

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes


Important

Refactor ShadowCheckpointService to improve performance and disk space management by caching nested git paths, optimizing file staging, and introducing garbage collection.

  • Performance Improvements:
    • Cache nested git repo paths in ShadowCheckpointService to avoid repeated scans.
    • Optimize stageAll() to use git status for specific file changes instead of git add ..
    • Modify renameNestedGitRepos() to use cached paths.
  • Garbage Collection:
    • Run git repack during initialization if a shadow repo exists.
    • Periodically run git gc after every 20 checkpoints in saveCheckpoint().
    • Run git gc --prune=now after deleting a branch in deleteBranch().
  • Diff Calculation:
    • Improve getDiff() to use git diff --name-status for precise change tracking and handle file content retrieval more accurately.

This description was created by Ellipsis for e72e069fa7d9ddc64f3c1de60f579450e136bec9. You can customize this summary. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented May 17, 2025

⚠️ No Changeset found

Latest commit: 4de36f9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 17, 2025
@hannesrudolph
Copy link
Collaborator

@gauravsaini would you be able to convert the PR to draft and generate and link to an issue please?

@gauravsaini gauravsaini force-pushed the checkpoint-perf-improvement branch from e72e069 to 43387b1 Compare May 18, 2025 02:46
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@gauravsaini gauravsaini changed the title WIP refactor:improve checkpoint and ensure gc to improve disk space refactor:improve checkpoint and ensure gc to improve disk space May 18, 2025
@gauravsaini
Copy link
Author

gauravsaini commented May 18, 2025

@hannesrudolph
Hey, I've added unit tests to make sure that it works fine . I've also added possible issues it might fix.
Ellipses dev bot gives a very good summary of the changes

@hannesrudolph
Copy link
Collaborator

@cte would you be able to review this before it gets outdated? Looks promising.

@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap May 20, 2025
@hannesrudolph hannesrudolph moved this from PR [Needs Review] to TEMP in Roo Code Roadmap May 26, 2025
@daniel-lxs daniel-lxs moved this from TEMP to PR [Needs Review] in Roo Code Roadmap May 27, 2025
@daniel-lxs
Copy link
Member

Hey @gauravsaini, thank you for your contribution, I really like your implementation.
I noticed some comments that probably need to be removed:
// --- CHANGE START:
// --- CHANGE END:
// --- ADDITION:

Since they don't add useful information to the code itself.

// Copied
filesToAdd.push(filePath) // Add the new path
}
// Other statuses like 'U' (unmerged) might need specific handling if relevant
Copy link
Member

@daniel-lxs daniel-lxs May 27, 2025

Choose a reason for hiding this comment

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

Hey @gauravsaini, quick thought on the new stageAll logic: how does it handle files that were part of a merge conflict and have been resolved in the working directory but not yet git added? The if/else chain doesn't seem to explicitly cover 'U' (unmerged) statuses. Just wondering if this might mean resolved conflicts aren't always included in checkpoints?

Copy link
Member

Choose a reason for hiding this comment

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

This seems like a non-issue, checkpoints won't have to deal with merge conflicts and much less with files that had merge conflicts at some point and then got the conflict solved.

@daniel-lxs
Copy link
Member

Hey @gauravsaini, it seems like the flag --path-walk used by your implementation:

await git.raw(["repack", "-a", "-d", "-f", "--path-walk", "--quiet"])

Isn't available on my git version 2.49.0

Do I need an specific version for this to work?

@ijin
Copy link

ijin commented Jun 7, 2025

@gauravsaini @daniel-lxs

After looking ways to reduce checkpoints, I stumbled upon this PR.

It looks like --path-walk is only available on Microsoft's version of git.
git-for-windows/git#5171

Since this option really shines for large scale repositories, maybe we could omit this for compatibility (I'm on a Mac), or use a similar but less efficient option like git repack -adf --window=250 ? Or alternatively fall back to it after the try catch.

ref: https://www.jonathancreamer.com/how-we-shrunk-our-git-repo-size-by-94-percent/

@daniel-lxs
Copy link
Member

maybe we could omit this for compatibility

Hey @ijin, it seems like the author of the PR is unavailable, if you'd like you can create a PR based on this one with the compatibility changes, let's see how that turns out.

Let me know if you have any questions!

@ijin
Copy link

ijin commented Jun 10, 2025

maybe we could omit this for compatibility

Hey @ijin, it seems like the author of the PR is unavailable, if you'd like you can create a PR based on this one with the compatibility changes, let's see how that turns out.

Let me know if you have any questions!

@daniel-lxs I took a stab at it!
#4494

SmartManoj pushed a commit to SmartManoj/Raa-Code that referenced this pull request Jun 13, 2025
…rtcuts in Cline (RooCodeInc#3695)

* Fix: Temporary revert protobus changes for Toggle plan and act mode

* Adding telemetry

* Adding telemetry

* Adding telemetry

* Adding telemetry

* Adding telemetry

---------

Co-authored-by: Cline Evaluation <[email protected]>
@hannesrudolph
Copy link
Collaborator

closed in favour of #4494

@github-project-automation github-project-automation bot moved this from PR [Changes Requested] to Done in Roo Code Roadmap Jun 16, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR - Changes Requested size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

tasks vanished (from log aswell) Tasks not loading Checkpoints creating excessive disk usage (40GB+) in VSCode global storage

4 participants