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

feat(kernel): wake tasks on service registration #268

Merged
merged 2 commits into from
Sep 3, 2023
Merged

Commits on Sep 3, 2023

  1. feat(kernel): wake tasks on service registration

    Depends on #267
    
    Currently, the `Registry::connect_*` methods return an error immediately
    if the requested service is not found in the registry. Most client types
    implement a `from_registry` method which retry in a loop when the
    requested service is not found in the registry. These methods will wait
    for a fixed (short) amount of time and then try again until the registry
    returns the service.
    
    This approach is quite inefficient, as we have to run a bunch of retry
    loops that keep trying to access a service that may not be there. This
    may happen several times before the service actually is registered,
    especially when registering a service requires connecting to another
    service.
    
    This branch improves the efficiency of waiting for a service to be
    registered. Now, rather than retrying with a fixed-duration sleep, we
    instead have the `Registry` own a `WaitCell` which is woken whenever a
    new service is registered. This wakes all takes potentially waiting to
    connect, allowing them to re-check whether the service they want is in
    the registry. This idea was initially proposed by @jamesmunns in a
    [comment] on PR #259
    
    Connections are now established using either `Registry::connect`, which
    retries whenever a new service is registered, or `Registry::try_connect`
    which never retries.
    
    Additionally, we now have the capacity to indicate that a service is not
    found *and* that the registry is full, by closing the `WaitCell`. In
    this case, retrying will never succeed, because the registry is full and
    if the service isn't already there, it will never be added. In this
    case, the retrying methods will also return an error, rather than never
    completing, so we avoid a potential task leak.
    
    In order to make this change, we need to move the `RwLock` from being
    around the entire `Registry` to being inside the registry, around
    `items`. This allows the `WaitCell` to be accessed regardless. It also
    allows us to shorten the duration for which the lock is held. This
    requires changing all methods on `Registry` to take `&self`.
    Therefore, I've removed the wrapper methods on `Kernel` for connecting
    and registering, since they can now just be called on `kernel.registry`
    without a bunch of extra boilerplate for lock management. I've also
    simplified the API surface of the registry a bit by removing the
    `connect` methods that don't take a `Hello`, and just using
    `Registry::connect(())` in those cases. IMO, those methods weren't
    really pulling their weight, and they required us to have a method named
    `Registry::try_connect_userspace_with_hello` if we were going to add a
    non-retrying `connect` variant. Now, we can just have
    `Registry::try_connect_userspace`, `Registry::connect_userspace`,
    `Registry::connect`, and `Registry::try_connect`, which feels much less
    egregious.
    
    [comment]: #258 (comment)
    hawkw committed Sep 3, 2023
    Configuration menu
    Copy the full SHA
    c4857ba View commit details
    Browse the repository at this point in the history
  2. fix bad eq impl

    hawkw committed Sep 3, 2023
    Configuration menu
    Copy the full SHA
    93e282c View commit details
    Browse the repository at this point in the history