Skip to content

Commit

Permalink
Cache xdg-desktop-portal appearance
Browse files Browse the repository at this point in the history
This improves the startup time.

Right now we query the portal appearance value again over dbus every
time that we access it, for example every time that the user calls
wezterm.gui.get_appearance() from the Lua interface.

Queries over dbus are slow, they usually take a few milliseconds to
complete, for example on my system a portal query over dbus takes around
2 milliseconds to complete.

Wezterm also automatically calls the portal during its own internal
x11/wayland connection initialization, thus right now wezterm queries
the appearance portal setting n+1 times on startup, where n is the
number of times that the user calls get_appearance() from the config.

To fix this problem, we simply cache the portal appearance.

Thus this patch decreases the startup time by 2ms for users that
configure wezterm to follow the global system theme and potentially by
more for users that call get_appearance() in inflational amounts.

Of course in order to prevent our cached value from going invalid, we
have to track the appearance value by subscribing to the SettingChanged
signal which we did already anyway.

refs: wez#2258
  • Loading branch information
vimpostor committed Aug 13, 2022
1 parent f6f36b4 commit c58a656
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 32 deletions.
8 changes: 7 additions & 1 deletion window/src/os/wayland/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::connection::ConnectionOps;
use crate::os::wayland::inputhandler::InputHandler;
use crate::os::wayland::output::OutputHandler;
use crate::os::x11::keyboard::Keyboard;
use crate::os::xdg_desktop_portal::XdgDesktopPortal;
use crate::screen::{ScreenInfo, Screens};
use crate::spawn::*;
use crate::{Appearance, Connection, ScreenRect, WindowEvent};
Expand Down Expand Up @@ -42,6 +43,7 @@ impl MyEnvironment {
}

pub struct WaylandConnection {
pub portal: XdgDesktopPortal,
should_terminate: RefCell<bool>,
pub(crate) next_window_id: AtomicUsize,
pub(crate) windows: RefCell<HashMap<usize, Rc<RefCell<WaylandWindowInner>>>>,
Expand Down Expand Up @@ -182,6 +184,9 @@ impl WaylandConnection {
let mem_pool = environment.create_auto_pool()?;

Ok(Self {
portal: XdgDesktopPortal {
appearance: RefCell::new(None),
},
display: RefCell::new(display),
environment,
should_terminate: RefCell::new(false),
Expand Down Expand Up @@ -414,6 +419,7 @@ impl WaylandConnection {
}

pub(crate) fn advise_of_appearance_change(&self, appearance: crate::Appearance) {
*self.portal.appearance.borrow_mut() = Some(appearance);
for win in self.windows.borrow().values() {
win.borrow_mut().appearance_changed(appearance);
}
Expand All @@ -426,7 +432,7 @@ impl ConnectionOps for WaylandConnection {
}

fn get_appearance(&self) -> Appearance {
match promise::spawn::block_on(crate::os::xdg_desktop_portal::get_appearance()) {
match promise::spawn::block_on(self.portal.get_appearance()) {
Ok(appearance) => return appearance,
Err(err) => {
log::debug!("Unable to resolve appearance using xdg-desktop-portal: {err:#}");
Expand Down
8 changes: 7 additions & 1 deletion window/src/os/x11/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::keyboard::Keyboard;
use crate::connection::ConnectionOps;
use crate::os::x11::window::XWindowInner;
use crate::os::x11::xsettings::*;
use crate::os::xdg_desktop_portal::XdgDesktopPortal;
use crate::os::Connection;
use crate::screen::{ScreenInfo, Screens};
use crate::spawn::*;
Expand All @@ -21,6 +22,7 @@ use xcb::{dri2, Raw};

pub struct XConnection {
pub conn: xcb::Connection,
pub portal: XdgDesktopPortal,
default_dpi: RefCell<f64>,
pub(crate) xsettings: RefCell<XSettingsMap>,
pub screen_num: i32,
Expand Down Expand Up @@ -125,7 +127,7 @@ impl ConnectionOps for XConnection {
}

fn get_appearance(&self) -> Appearance {
match promise::spawn::block_on(crate::os::xdg_desktop_portal::get_appearance()) {
match promise::spawn::block_on(self.portal.get_appearance()) {
Ok(appearance) => return appearance,
Err(err) => {
log::debug!("Unable to resolve appearance using xdg-desktop-portal: {err:#}");
Expand Down Expand Up @@ -320,6 +322,7 @@ impl XConnection {
}

pub(crate) fn advise_of_appearance_change(&self, appearance: crate::Appearance) {
*self.portal.appearance.borrow_mut() = Some(appearance);
for win in self.windows.borrow().values() {
win.lock().unwrap().appearance_changed(appearance);
}
Expand Down Expand Up @@ -564,6 +567,9 @@ impl XConnection {

let conn = Rc::new(XConnection {
conn,
portal: XdgDesktopPortal {
appearance: RefCell::new(None),
},
default_dpi,
xsettings: RefCell::new(xsettings),
cursor_font_id,
Expand Down
11 changes: 8 additions & 3 deletions window/src/os/x_and_wayland.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,16 @@ impl ConnectionOps for Connection {
}

fn run_message_loop(&self) -> anyhow::Result<()> {
crate::os::xdg_desktop_portal::subscribe();
match self {
Self::X11(x) => x.run_message_loop(),
Self::X11(x) => {
x.portal.subscribe();
x.run_message_loop()
}
#[cfg(feature = "wayland")]
Self::Wayland(w) => w.run_message_loop(),
Self::Wayland(w) => {
w.portal.subscribe();
w.run_message_loop()
}
}
}

Expand Down
76 changes: 49 additions & 27 deletions window/src/os/xdg_desktop_portal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use crate::{Appearance, Connection, ConnectionOps};
use anyhow::Context;
use futures_util::stream::StreamExt;
use std::cell::RefCell;
use std::collections::HashMap;
use zbus::dbus_proxy;
use zvariant::OwnedValue;
Expand All @@ -26,6 +27,54 @@ trait PortalSettings {
fn SettingChanged(&self, namespace: &str, key: &str, value: OwnedValue) -> Result<()>;
}

pub struct XdgDesktopPortal {
pub appearance: RefCell<Option<Appearance>>,
}

impl XdgDesktopPortal {
pub async fn get_appearance(&self) -> anyhow::Result<Appearance> {
let mut appearance = self.appearance.borrow_mut();
match *appearance {
Some(a) => {
// return cached appearance
Ok(a)
}
None => {
let value = read_setting("org.freedesktop.appearance", "color-scheme").await?;
match value_to_appearance(value) {
Ok(a) => {
*appearance = Some(a);
Ok(a)
}
err => err,
}
}
}
}

pub fn subscribe(&self) {
promise::spawn::spawn(async move {
let connection = zbus::ConnectionBuilder::session()?.build().await?;
let proxy = PortalSettingsProxy::new(&connection)
.await
.context("make proxy")?;
let mut stream = proxy.receive_SettingChanged().await?;
while let Some(signal) = stream.next().await {
let args = signal.args()?;
if args.namespace == "org.freedesktop.appearance" && args.key == "color-scheme" {
if let Ok(appearance) = value_to_appearance(args.value) {
let conn = Connection::get()
.ok_or_else(|| anyhow::anyhow!("connection is dead"))?;
conn.advise_of_appearance_change(appearance);
}
}
}
Result::<(), anyhow::Error>::Ok(())
})
.detach();
}
}

pub async fn read_setting(namespace: &str, key: &str) -> anyhow::Result<OwnedValue> {
let connection = zbus::ConnectionBuilder::session()?.build().await?;
let proxy = PortalSettingsProxy::new(&connection)
Expand All @@ -46,30 +95,3 @@ fn value_to_appearance(value: OwnedValue) -> anyhow::Result<Appearance> {
}
})
}

pub async fn get_appearance() -> anyhow::Result<Appearance> {
let value = read_setting("org.freedesktop.appearance", "color-scheme").await?;
value_to_appearance(value)
}

pub fn subscribe() {
promise::spawn::spawn(async move {
let connection = zbus::ConnectionBuilder::session()?.build().await?;
let proxy = PortalSettingsProxy::new(&connection)
.await
.context("make proxy")?;
let mut stream = proxy.receive_SettingChanged().await?;
while let Some(signal) = stream.next().await {
let args = signal.args()?;
if args.namespace == "org.freedesktop.appearance" && args.key == "color-scheme" {
if let Ok(appearance) = value_to_appearance(args.value) {
let conn =
Connection::get().ok_or_else(|| anyhow::anyhow!("connection is dead"))?;
conn.advise_of_appearance_change(appearance);
}
}
}
Result::<(), anyhow::Error>::Ok(())
})
.detach();
}

0 comments on commit c58a656

Please sign in to comment.