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.

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

Keep a cache of session file data, and only write if changes occur #1316

Closed

Conversation

kannibalox
Copy link
Contributor

This is mainly in response to #1313, but making session.save less heavy in general has been something I've thought about, and this approach seemed simpler than trying to mark downloads as dirty or have logic depending on the state as described here. The overhead of a cache miss is pretty low, but rakshasa/libtorrent#257 was mainly to make this much more effective for libtorrent_resume files. I'm on the fence about whether this is an elegant use of cache or a terrible hack, so I submit it for your consideration.

It includes two other changes to behavior:

  • It's now possible for .libtorrent_resume to get updated while .rtorrent doesn't. I couldn't think of a scenario where this would be bad, and it would've complicated error handling, but it can be done if desired.
  • An ::access() call is made on a cache hit. This is primarily to support the rare but important use case of being able to restore a session directory entirely from scratch on a running instance (e.g. if someone accidentally deletes it)

Something to consider might be adding an override flag so that d.save_full_session and d.save_resume can ignore the cache.

@rakshasa
Copy link
Owner

There's no reason why we should need to do work in the main thread, we only need to pass encoded bencode blobs to a worker thread and have it handle everything.

As such I don't think cache's should be added before that is done.

@kannibalox
Copy link
Contributor Author

Makes sense, I'll close this for now and can revisit it when things are threaded.

@kannibalox kannibalox closed this Nov 14, 2024
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.

2 participants