update embedded-graphics to 0.8 everywhere #259
Comments
hawkw
added a commit
that referenced
this issue
Sep 3, 2023
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
added a commit
that referenced
this issue
Sep 3, 2023
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
added a commit
that referenced
this issue
Sep 3, 2023
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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: