Skip to content

Commit

Permalink
perf: replace unnecessary Arcs with Rc
Browse files Browse the repository at this point in the history
Now that DeviceRegistry and Device are no longer Send + Sync, there is
no reason for them to use Arc over Rc.

Also replaces tokio spawn_local usage with spawn to avoid having to
deal with LocalSets.
  • Loading branch information
Oppzippy committed Aug 5, 2023
1 parent 1f258e5 commit dc7c590
Show file tree
Hide file tree
Showing 29 changed files with 90 additions and 91 deletions.
4 changes: 2 additions & 2 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{error::Error, sync::Arc};
use std::{error::Error, rc::Rc};

use clap::Parser;
use cli::{Cli, Command};
Expand Down Expand Up @@ -59,7 +59,7 @@ async fn do_cli_command(args: Cli, registry: impl DeviceRegistry) -> Result<(),
async fn get_device_or_err<T>(
registry: &T,
descriptor: &T::DescriptorType,
) -> Result<Arc<T::DeviceType>, String>
) -> Result<Rc<T::DeviceType>, String>
where
T: DeviceRegistry,
{
Expand Down
2 changes: 1 addition & 1 deletion gui/src/actions/create_custom_equalizer_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub fn create_custom_equalizer_profile<T>(
custom_profile: &CustomEqualizerProfileObject,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
settings_file.edit(|settings| {
settings.set_custom_profile(
Expand Down
2 changes: 1 addition & 1 deletion gui/src/actions/delete_custom_equalizer_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub fn delete_custom_equalizer_profile<T>(
custom_profile: &CustomEqualizerProfileObject,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
settings_file.edit(|settings| {
settings.remove_custom_profile(&custom_profile.name());
Expand Down
2 changes: 1 addition & 1 deletion gui/src/actions/refresh_devices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use super::{State, StateUpdate};

pub async fn refresh_devices<T>(state: &Rc<State<T>>) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
if !state.is_refresh_in_progress.get() {
let descriptors = {
Expand Down
2 changes: 1 addition & 1 deletion gui/src/actions/select_custom_equalizer_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub async fn select_custom_equalizer_configuration<T>(
custom_profile: &CustomEqualizerProfileObject,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
settings_file
.get(|settings| {
Expand Down
6 changes: 3 additions & 3 deletions gui/src/actions/set_ambient_sound_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub async fn set_ambient_sound_mode<T>(
ambient_sound_mode: AmbientSoundMode,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
let device = state
.selected_device()
Expand All @@ -34,7 +34,7 @@ where

#[cfg(test)]
mod tests {
use std::sync::Arc;
use std::rc::Rc;

use mockall::predicate;
use openscq30_lib::{
Expand Down Expand Up @@ -72,7 +72,7 @@ mod tests {
sound_modes.ambient_sound_mode == AmbientSoundMode::NoiseCanceling
}))
.return_once(|_ambient_sound_mode| Ok(()));
*state.selected_device.borrow_mut() = Some(Arc::new(selected_device));
*state.selected_device.borrow_mut() = Some(Rc::new(selected_device));

set_ambient_sound_mode(&state, AmbientSoundMode::NoiseCanceling)
.await
Expand Down
6 changes: 3 additions & 3 deletions gui/src/actions/set_custom_noise_canceling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub async fn set_custom_noise_canceling<T>(
custom_noise_canceling: CustomNoiseCanceling,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
if let Some(handle) = state.set_custom_noise_canceling_handle.take() {
handle.abort();
Expand Down Expand Up @@ -61,7 +61,7 @@ where

#[cfg(test)]
mod tests {
use std::sync::Arc;
use std::rc::Rc;

use mockall::predicate;
use openscq30_lib::{
Expand Down Expand Up @@ -99,7 +99,7 @@ mod tests {
sound_modes.custom_noise_canceling == CustomNoiseCanceling::new(2)
}))
.return_once(|_custom_noise_canceling| Ok(()));
*state.selected_device.borrow_mut() = Some(Arc::new(selected_device));
*state.selected_device.borrow_mut() = Some(Rc::new(selected_device));

set_custom_noise_canceling(&state, CustomNoiseCanceling::new(2))
.await
Expand Down
8 changes: 4 additions & 4 deletions gui/src/actions/set_device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub async fn set_device<T>(
mac_address: Option<MacAddr6>,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
// Clean up any existing devices
if let Some(handle) = &*state.connect_to_device_handle.borrow_mut() {
Expand Down Expand Up @@ -113,7 +113,7 @@ where

#[cfg(test)]
mod tests {
use std::{collections::VecDeque, sync::Arc};
use std::{collections::VecDeque, rc::Rc};

use macaddr::MacAddr6;
use mockall::predicate;
Expand Down Expand Up @@ -179,7 +179,7 @@ mod tests {
.return_const(receiver);
device.expect_state().once().return_const(device_state_2);

Ok(Some(Arc::new(device)))
Ok(Some(Rc::new(device)))
});

let (state, mut receiver) = State::new(registry);
Expand Down Expand Up @@ -268,7 +268,7 @@ mod tests {
..Default::default()
});

Ok(Some(Arc::new(device)))
Ok(Some(Rc::new(device)))
});

let (state, _receiver) = State::new(registry);
Expand Down
10 changes: 5 additions & 5 deletions gui/src/actions/set_equalizer_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub async fn set_equalizer_configuration<T>(
equalizer_configuration: impl Into<EqualizerConfiguration>,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
set_equalizer_configuration_impl(state, equalizer_configuration.into()).await
}
Expand All @@ -25,7 +25,7 @@ async fn set_equalizer_configuration_impl<T>(
equalizer_configuration: EqualizerConfiguration,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
if let Some(handle) = state.set_equalizer_configuration_handle.take() {
handle.abort();
Expand Down Expand Up @@ -64,7 +64,7 @@ where

#[cfg(test)]
mod tests {
use std::{sync::Arc, time::Duration};
use std::{rc::Rc, time::Duration};

use gtk::glib::{clone, timeout_future, MainContext};
use mockall::predicate;
Expand All @@ -90,7 +90,7 @@ mod tests {
EqualizerConfiguration::new_from_preset_profile(PresetEqualizerProfile::Acoustic),
)))
.return_once(|_ambient_sound_mode| Ok(()));
*state.selected_device.borrow_mut() = Some(Arc::new(selected_device));
*state.selected_device.borrow_mut() = Some(Rc::new(selected_device));

set_equalizer_configuration(
&state,
Expand All @@ -113,7 +113,7 @@ mod tests {
EqualizerConfiguration::new_from_preset_profile(PresetEqualizerProfile::Acoustic),
)))
.return_once(|_ambient_sound_mode| Ok(()));
*state.selected_device.borrow_mut() = Some(Arc::new(selected_device));
*state.selected_device.borrow_mut() = Some(Rc::new(selected_device));

MainContext::default().spawn_local(clone!(@strong state => async move {
set_equalizer_configuration(
Expand Down
6 changes: 3 additions & 3 deletions gui/src/actions/set_noise_canceling_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub async fn set_noise_canceling_mode<T>(
noise_canceling_mode: NoiseCancelingMode,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
let device = state
.selected_device()
Expand All @@ -34,7 +34,7 @@ where

#[cfg(test)]
mod tests {
use std::sync::Arc;
use std::rc::Rc;

use mockall::predicate;
use openscq30_lib::{
Expand Down Expand Up @@ -72,7 +72,7 @@ mod tests {
sound_modes.noise_canceling_mode == NoiseCancelingMode::Transport
}))
.return_once(|_noise_canceling_mode| Ok(()));
*state.selected_device.borrow_mut() = Some(Arc::new(selected_device));
*state.selected_device.borrow_mut() = Some(Rc::new(selected_device));

set_noise_canceling_mode(&state, NoiseCancelingMode::Transport)
.await
Expand Down
6 changes: 3 additions & 3 deletions gui/src/actions/set_transpareny_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub async fn set_transparency_mode<T>(
transparency_mode: TransparencyMode,
) -> anyhow::Result<()>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
let device = state
.selected_device()
Expand All @@ -34,7 +34,7 @@ where

#[cfg(test)]
mod tests {
use std::sync::Arc;
use std::rc::Rc;

use mockall::predicate;
use openscq30_lib::{
Expand Down Expand Up @@ -72,7 +72,7 @@ mod tests {
sound_modes.transparency_mode == TransparencyMode::VocalMode
}))
.return_once(|_transparency_mode| Ok(()));
*state.selected_device.borrow_mut() = Some(Arc::new(selected_device));
*state.selected_device.borrow_mut() = Some(Rc::new(selected_device));

set_transparency_mode(&state, TransparencyMode::VocalMode)
.await
Expand Down
9 changes: 4 additions & 5 deletions gui/src/actions/state.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{
cell::{Cell, RefCell},
rc::Rc,
sync::Arc,
};

use gtk::glib::JoinHandle;
Expand All @@ -14,11 +13,11 @@ use super::StateUpdate;

pub struct State<T>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
pub state_update_sender: UnboundedSender<StateUpdate>,
pub registry: T,
pub selected_device: RefCell<Option<Arc<T::DeviceType>>>,
pub selected_device: RefCell<Option<Rc<T::DeviceType>>>,
pub connect_to_device_handle: RefCell<Option<JoinHandle<()>>>,
pub set_equalizer_configuration_handle: RefCell<Option<JoinHandle<()>>>,
pub set_custom_noise_canceling_handle: RefCell<Option<JoinHandle<()>>>,
Expand All @@ -28,7 +27,7 @@ where

impl<T> State<T>
where
T: DeviceRegistry + Send + Sync + 'static,
T: DeviceRegistry + 'static,
{
pub fn new(registry: T) -> (Rc<Self>, UnboundedReceiver<StateUpdate>) {
let (sender, receiver) = mpsc::unbounded_channel::<StateUpdate>();
Expand All @@ -47,7 +46,7 @@ where
)
}

pub fn selected_device(&self) -> Option<Arc<T::DeviceType>> {
pub fn selected_device(&self) -> Option<Rc<T::DeviceType>> {
self.selected_device.borrow().clone()
}
}
8 changes: 4 additions & 4 deletions gui/src/gtk_openscq30_lib/gtk_device.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fmt::Debug, sync::Arc};
use std::{fmt::Debug, rc::Rc};

use async_trait::async_trait;
use macaddr::MacAddr6;
Expand All @@ -18,15 +18,15 @@ pub struct GtkDevice<InnerDeviceType: 'static>
where
InnerDeviceType: Device,
{
tokio_runtime: Arc<Runtime>,
soundcore_device: Arc<InnerDeviceType>,
tokio_runtime: Rc<Runtime>,
soundcore_device: Rc<InnerDeviceType>,
}

impl<InnerDeviceType> GtkDevice<InnerDeviceType>
where
InnerDeviceType: Device,
{
pub fn new(device: Arc<InnerDeviceType>, tokio_runtime: Arc<Runtime>) -> Self {
pub fn new(device: Rc<InnerDeviceType>, tokio_runtime: Rc<Runtime>) -> Self {
Self {
tokio_runtime,
soundcore_device: device,
Expand Down
18 changes: 9 additions & 9 deletions gui/src/gtk_openscq30_lib/gtk_device_registry.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::sync::Arc;
use std::rc::Rc;

use macaddr::MacAddr6;
use openscq30_lib::api::device::DeviceRegistry;
Expand All @@ -11,8 +11,8 @@ pub struct GtkDeviceRegistry<InnerRegistryType>
where
InnerRegistryType: DeviceRegistry,
{
tokio_runtime: Arc<Runtime>,
soundcore_device_registry: Arc<InnerRegistryType>,
tokio_runtime: Rc<Runtime>,
soundcore_device_registry: Rc<InnerRegistryType>,
}

#[async_trait(?Send)]
Expand All @@ -26,7 +26,7 @@ where
async fn device(
&self,
mac_address: MacAddr6,
) -> openscq30_lib::Result<Option<Arc<Self::DeviceType>>> {
) -> openscq30_lib::Result<Option<Rc<Self::DeviceType>>> {
let mac_address = mac_address.to_owned();
let device_registry = self.soundcore_device_registry.to_owned();
let maybe_device = self
Expand Down Expand Up @@ -56,15 +56,15 @@ where
{
pub fn new(registry: InnerRegistryType, tokio_runtime: Runtime) -> Self {
Self {
soundcore_device_registry: Arc::new(registry),
tokio_runtime: Arc::new(tokio_runtime),
soundcore_device_registry: Rc::new(registry),
tokio_runtime: Rc::new(tokio_runtime),
}
}

fn to_gtk_device(
&self,
device: Arc<InnerRegistryType::DeviceType>,
) -> Arc<GtkDevice<InnerRegistryType::DeviceType>> {
Arc::new(GtkDevice::new(device, self.tokio_runtime.to_owned()))
device: Rc<InnerRegistryType::DeviceType>,
) -> Rc<GtkDevice<InnerRegistryType::DeviceType>> {
Rc::new(GtkDevice::new(device, self.tokio_runtime.to_owned()))
}
}
4 changes: 2 additions & 2 deletions gui/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ fn build_ui(application: &adw::Application) {

fn build_ui_2(
application: &adw::Application,
gtk_registry: GtkDeviceRegistry<impl DeviceRegistry + Send + Sync + 'static>,
gtk_registry: GtkDeviceRegistry<impl DeviceRegistry + 'static>,
) {
#[cfg(target_os = "windows")]
if let Err(err) = set_ui_theme(application) {
Expand Down Expand Up @@ -235,7 +235,7 @@ fn build_ui_2(

fn handle_error<T>(err: anyhow::Error, state: &State<T>)
where
T: DeviceRegistry + Send + Sync,
T: DeviceRegistry,
{
let deselect_device = || {
state
Expand Down
6 changes: 3 additions & 3 deletions gui/src/mock/device_registry.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{sync::Arc, time::Duration};
use std::{rc::Rc, time::Duration};

use async_trait::async_trait;
use gtk::glib::timeout_future;
Expand All @@ -11,7 +11,7 @@ use super::MockDevice;
mock! {
pub DeviceRegistry {
pub fn device_descriptors(&self) -> openscq30_lib::Result<Vec<GenericDeviceDescriptor>>;
pub fn device(&self, mac_address: MacAddr6) -> openscq30_lib::Result<Option<Arc<MockDevice>>>;
pub fn device(&self, mac_address: MacAddr6) -> openscq30_lib::Result<Option<Rc<MockDevice>>>;
}
}

Expand All @@ -28,7 +28,7 @@ impl DeviceRegistry for MockDeviceRegistry {
async fn device(
&self,
mac_address: MacAddr6,
) -> openscq30_lib::Result<Option<Arc<Self::DeviceType>>> {
) -> openscq30_lib::Result<Option<Rc<Self::DeviceType>>> {
timeout_future(Duration::from_millis(10)).await;
self.device(mac_address)
}
Expand Down
Loading

0 comments on commit dc7c590

Please sign in to comment.