Skip to content

Conversation

rklaehn
Copy link
Collaborator

@rklaehn rklaehn commented Jul 9, 2025

Description

In the current blobs I am using tokio::sync::watch for notifying the observers when the state for a hash has changed. This works, but is way more heavy than needed.

  1. tokio::sync::watch is optimized for a case where we have tons of watchers. It uses a giant BigNotify under the hood to solve the thundering herd problem. I think it will be much more common that you have a few tasks operating on a hash (frequently just one), and a few observers (frequently none).
  2. since we now know that all tasks for a hash run concurrently but not parallel, we can use the awesome AtomicRefCell instead of a full blown lock.

Breaking Changes

None

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

rklaehn added 19 commits July 8, 2025 10:05
will probably inline this once it is proven to work well
futures-buffered is reexported by n0-future, as I learned yesterday
...by reverting a change that I did because of some older version of clippy
Since we are guaranteed to be single threaded within the handler fns for a
single hash, we can use a much more lightweight watcher impl.

Also, tokio::sync::watch is optimized for the case where there are a lot
of parallel watchers, using a giant BigNotify internally. We want to
optimize for the case where there are just 0 or a few watchers, so we are
not that much concerned with the thundering herd problem.
Copy link

github-actions bot commented Jul 9, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-blobs/pr/101/docs/iroh_blobs/

Last updated: 2025-08-05T10:40:09Z

@rklaehn rklaehn mentioned this pull request Jul 9, 2025
4 tasks
_state: entity_manager::ActiveEntityState<Self>,
_cause: entity_manager::ShutdownCause,
) {
async fn on_shutdown(state: HashContext, cause: ShutdownCause) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unrelated change!

@n0bot n0bot bot added this to iroh Jul 9, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Jul 9, 2025
@rklaehn
Copy link
Collaborator Author

rklaehn commented Jul 9, 2025

Did a very superficial bench with the linux kernel. Doesn't seem to make perf worse, but also not better (not that I expected that...)

@rklaehn rklaehn closed this Jul 9, 2025
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Jul 9, 2025
@rklaehn rklaehn reopened this Jul 9, 2025
Base automatically changed from entity-manager to main August 5, 2025 10:31
@rklaehn rklaehn marked this pull request as ready for review August 5, 2025 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

1 participant