From d32746b0e6bc5b171a0ddcac5983f47c8ae2ff26 Mon Sep 17 00:00:00 2001 From: "Jan Alexander Steffens (heftig)" Date: Thu, 9 Feb 2023 16:02:01 +0100 Subject: [PATCH] glib: Make WeakRef cheaply cloneable By using an `Arc` instead of a `Box`, we can make `WeakRef::clone` a cheap operation, compared to taking a global write lock for setting a `GWeakRef`. The extra space used for the refcounts should be a fine trade-off. --- glib/src/object.rs | 71 +++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/glib/src/object.rs b/glib/src/object.rs index 6588d9b79e71..b146594fd050 100644 --- a/glib/src/object.rs +++ b/glib/src/object.rs @@ -3,7 +3,15 @@ // rustdoc-stripper-ignore-next //! `IMPL` Object wrapper implementation and `Object` binding. -use std::{cmp, fmt, hash, marker::PhantomData, mem, mem::ManuallyDrop, ops, pin::Pin, ptr}; +use std::{ + cmp, fmt, hash, + marker::PhantomData, + mem::{self, ManuallyDrop}, + ops, + pin::Pin, + ptr, + sync::Arc, +}; use crate::{ closure::TryFromClosureReturnValue, @@ -3029,9 +3037,12 @@ impl ObjectExt for T { #[inline] fn downgrade(&self) -> WeakRef { unsafe { - let w = WeakRef(Box::pin(mem::zeroed()), PhantomData); + let w = WeakRef(Arc::pin(WeakRefInner { + ffi: mem::zeroed(), + phantom: PhantomData, + })); gobject_ffi::g_weak_ref_init( - mut_override(&*w.0), + mut_override(&w.0.ffi), self.as_object_ref().to_glib_none().0, ); w @@ -3373,9 +3384,15 @@ impl WeakRefNotify { // rustdoc-stripper-ignore-next /// A weak reference to an object. -#[derive(Debug)] +#[derive(Debug, PartialEq, PartialOrd)] #[doc(alias = "GWeakRef")] -pub struct WeakRef(Pin>, PhantomData<*mut T>); +pub struct WeakRef(Pin>>); + +#[derive(Debug)] +struct WeakRefInner { + ffi: gobject_ffi::GWeakRef, + phantom: PhantomData<*mut T>, +} impl WeakRef { // rustdoc-stripper-ignore-next @@ -3385,11 +3402,11 @@ impl WeakRef { #[inline] pub fn new() -> WeakRef { unsafe { - let mut w = WeakRef(Box::pin(mem::zeroed()), PhantomData); - gobject_ffi::g_weak_ref_init( - Pin::as_mut(&mut w.0).get_unchecked_mut(), - ptr::null_mut(), - ); + let w = WeakRef(Arc::pin(WeakRefInner { + ffi: mem::zeroed(), + phantom: PhantomData, + })); + gobject_ffi::g_weak_ref_init(mut_override(&w.0.ffi), ptr::null_mut()); w } } @@ -3401,7 +3418,7 @@ impl WeakRef { pub fn set(&self, obj: Option<&T>) { unsafe { gobject_ffi::g_weak_ref_set( - mut_override(Pin::as_ref(&self.0).get_ref()), + mut_override(&self.0.ffi), obj.map_or(std::ptr::null_mut(), |obj| { obj.as_object_ref().to_glib_none().0 }), @@ -3417,7 +3434,7 @@ impl WeakRef { #[inline] pub fn upgrade(&self) -> Option { unsafe { - let ptr = gobject_ffi::g_weak_ref_get(mut_override(Pin::as_ref(&self.0).get_ref())); + let ptr = gobject_ffi::g_weak_ref_get(mut_override(&self.0.ffi)); if ptr.is_null() { None } else { @@ -3428,11 +3445,11 @@ impl WeakRef { } } -impl Drop for WeakRef { +impl Drop for WeakRefInner { #[inline] fn drop(&mut self) { unsafe { - gobject_ffi::g_weak_ref_clear(Pin::as_mut(&mut self.0).get_unchecked_mut()); + gobject_ffi::g_weak_ref_clear(&mut self.ffi); } } } @@ -3440,17 +3457,7 @@ impl Drop for WeakRef { impl Clone for WeakRef { #[inline] fn clone(&self) -> Self { - unsafe { - let o = self.upgrade(); - - let mut c = WeakRef(Box::pin(mem::zeroed()), PhantomData); - gobject_ffi::g_weak_ref_init( - Pin::as_mut(&mut c.0).get_unchecked_mut(), - o.to_glib_none().0 as *mut gobject_ffi::GObject, - ); - - c - } + Self(self.0.clone()) } } @@ -3461,27 +3468,27 @@ impl Default for WeakRef { } } -unsafe impl Sync for WeakRef {} -unsafe impl Send for WeakRef {} +unsafe impl Sync for WeakRefInner {} +unsafe impl Send for WeakRefInner {} -impl PartialEq for WeakRef { +impl PartialEq for WeakRefInner { #[inline] fn eq(&self, other: &Self) -> bool { - unsafe { self.0.priv_.p == other.0.priv_.p } + unsafe { self.ffi.priv_.p == other.ffi.priv_.p } } } impl PartialEq for WeakRef { #[inline] fn eq(&self, other: &T) -> bool { - unsafe { self.0.priv_.p == other.as_ptr() as *mut std::os::raw::c_void } + unsafe { self.0.ffi.priv_.p == other.as_ptr() as *mut std::os::raw::c_void } } } -impl PartialOrd for WeakRef { +impl PartialOrd for WeakRefInner { #[inline] fn partial_cmp(&self, other: &Self) -> Option { - unsafe { self.0.priv_.p.partial_cmp(&other.0.priv_.p) } + unsafe { self.ffi.priv_.p.partial_cmp(&other.ffi.priv_.p) } } }