Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

some way to refresh in-memory packed refs cache without relying on mtime #1348

Closed
yuja opened this issue Apr 21, 2024 · 2 comments
Closed

some way to refresh in-memory packed refs cache without relying on mtime #1348

yuja opened this issue Apr 21, 2024 · 2 comments
Assignees
@Byron
Labels
enhancement New feature or request

Comments

@yuja
Copy link
Contributor

yuja commented Apr 21, 2024

Summary 💡

Does it make sense to add API to trigger RefStore::force_refresh_packed_buffer() or make it public?

Motivation 🔦

In jj, gc is implemented as a combination of internal ref cleanup through gix API + shelling out git gc command. Since git gc will move loose refs to packed-refs, I want to ensure that the in-memory packed cache is invalidated. mtime isn't reliable if git gc is super fast.

https://github.com/martinvonz/jj/blob/77eaf67f96c6ea438e2dd1a5636f34b6a321e6be/lib/src/git_backend.rs#L1213
@yuja yuja added the enhancement New feature or request label Apr 21, 2024
@Byron Byron self-assigned this Apr 22, 2024
Byron added a commit that referenced this issue Apr 22, 2024
@Byron
feat: add file::Store::force_refresh_packed_buffer() to public API (#…
f2d8955 
…1348)

That way it's possible to explicitly refresh if the caller knows that some other
operation might have invalidated the in-memory cache. The `mtime` based approach
doesn't work reliably on all filesystems due to coarse granularity of time.
@Byron
Copy link
Member

Byron commented Apr 22, 2024 •
edited
Loading

Thanks for bringing this up!

I have implemented this in f2d8955 which will definitely be merged within a month or less. Edit: It will be merged very soon to fix the cargo-deny based CI failure as well.

@Byron
Copy link
Member

Byron commented Apr 22, 2024

Closing, as the branch was merged and the method will be available with the next release. Please feel free to leave additional comments or reopen as you see fit.

@Byron Byron closed this as completed Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Byron Byron

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Byron @yuja