From 8984c2e0597536e977997cfd8a107ada1a8d8d84 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Mon, 10 Nov 2025 18:10:27 +0100 Subject: [PATCH] Dispatcher: Reduce operations by moving things into a single Arc Before dropping a `DispatchGuard` required invoking drop on 2 `Arc`s (reducing their reference count) and 3 `Sender`. That happens for every single `launch` on the global dispatcher (because that clones a new guard). That's a bit overkill. `Sender` is already `Sync` (when its `T` is `Send`), that means it's enough if we keep one of them around (behind the outer `Arc`, our `T` is the launched function with an explicit `Send` bound). Before this the drop of `DispatchGuard` shows up in profiles for a synthetic benchmark (7.5% for that drop, with 3.4% attributed to dropping `Sender`). With this patch that overhead seems to be gone. --- CHANGELOG.md | 1 + glean-core/src/dispatcher/mod.rs | 26 ++++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 698939bad4..9a30d2702c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * General * Stop reporting db file sizes during init phase ([#3331](https://github.com/mozilla/glean/pull/3331)) + * Tiny performance improvement for putting tasks on the dispatcher ([#3318](https://github.com/mozilla/glean/pull/3318)) # v66.1.1 (2025-11-06) diff --git a/glean-core/src/dispatcher/mod.rs b/glean-core/src/dispatcher/mod.rs index 2b5c08314c..c7c604e17b 100644 --- a/glean-core/src/dispatcher/mod.rs +++ b/glean-core/src/dispatcher/mod.rs @@ -24,6 +24,7 @@ use std::{ mem, + ops::Deref, sync::{ atomic::{AtomicBool, AtomicUsize, Ordering}, Arc, @@ -92,11 +93,23 @@ impl From> for DispatchError { /// A clonable guard for a dispatch queue. #[derive(Clone)] struct DispatchGuard { + inner: Arc, +} + +impl Deref for DispatchGuard { + type Target = DispatchGuardInner; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +struct DispatchGuardInner { /// Whether to queue on the preinit buffer or on the unbounded queue - queue_preinit: Arc, + queue_preinit: AtomicBool, /// The number of items that were added to the queue after it filled up. - overflow_count: Arc, + overflow_count: AtomicUsize, /// The maximum pre-init queue size max_queue_size: usize, @@ -262,8 +275,8 @@ impl Dispatcher { let (preinit_sender, preinit_receiver) = unbounded(); let (sender, mut unbounded_receiver) = unbounded(); - let queue_preinit = Arc::new(AtomicBool::new(true)); - let overflow_count = Arc::new(AtomicUsize::new(0)); + let queue_preinit = AtomicBool::new(true); + let overflow_count = AtomicUsize::new(0); let worker = crate::thread::spawn("glean.dispatcher", move || { match block_receiver.recv() { @@ -322,14 +335,15 @@ impl Dispatcher { }) .expect("Failed to spawn Glean's dispatcher thread"); - let guard = DispatchGuard { + let inner = Arc::new(DispatchGuardInner { queue_preinit, overflow_count, max_queue_size, block_sender, preinit_sender, sender, - }; + }); + let guard = DispatchGuard { inner }; Dispatcher { guard,