From 95e36249d51bb1d31e84e38bbfe46a95e8f33006 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 8 Jun 2023 17:45:54 +0200 Subject: [PATCH] Remove the dependency on ouroboros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reduces compilation time by removing a dependency on syn and other dependencies of ouroboros_macros. In addition it saves a lot of unused codegened methods. On my laptop (2 core + HT) this reduces compilation time by ~20%. On a many core system this doesn't help much though as the critical path path consists of both ttf-parser -> rustybuzz and swash. Further gains will likely need to be made by reducing compilation time for these crates. Benchmark 1: cargo build Time (mean ± σ): 25.150 s ± 0.167 s [User: 84.414 s, System: 7.335 s] Range (min … max): 24.909 s … 25.444 s 10 runs Benchmark 1: cargo build Time (mean ± σ): 19.819 s ± 0.226 s [User: 67.754 s, System: 5.592 s] Range (min … max): 19.492 s … 20.140 s 10 runs The code is based on an expansion of the ouroboros macro, cleaned up to remove all unused methods and inline most functions that are only called once. --- Cargo.toml | 3 +- src/font/mod.rs | 168 ++++++++++++++++++++++++++++-------------------- 2 files changed, 101 insertions(+), 70 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a5f02cf..a818ed4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ repository = "https://github.com/pop-os/cosmic-text" fontdb = { version = "0.13.0", default-features = false } libm = "0.2.6" log = "0.4.17" -ouroboros = { version = "0.15.5", default-features = false } +aliasable = "0.1.3" rustybuzz = { version = "0.7.0", default-features = false, features = ["libm"] } swash = { version = "0.1.6", optional = true } syntect = { version = "5.0.0", optional = true } @@ -35,7 +35,6 @@ no_std = [ std = [ "fontdb/memmap", "fontdb/std", - "ouroboros/std", "rustybuzz/std", "sys-locale", "unicode-bidi/std", diff --git a/src/font/mod.rs b/src/font/mod.rs index ea7b9be..9c277a5 100644 --- a/src/font/mod.rs +++ b/src/font/mod.rs @@ -1,32 +1,104 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 pub(crate) mod fallback; +use alloc::boxed::Box; use alloc::sync::Arc; pub use self::system::*; mod system; -/// A font -pub struct Font(FontInner); +pub use font_inner::Font; -#[ouroboros::self_referencing] -#[allow(dead_code)] -struct FontInner { - id: fontdb::ID, - data: Arc + Send + Sync>, - #[borrows(data)] - #[covariant] - rustybuzz: rustybuzz::Face<'this>, - // workaround, since ouroboros does not work with #[cfg(feature = "swash")] - swash: SwashKey, +/// Encapsulates the self-referencing `Font` struct to ensure all field accesses have to go through +/// safe methods. +mod font_inner { + use super::*; + use aliasable::boxed::AliasableBox; + + /// A font + // + // # Safety invariant + // + // `data` must never have a mutable reference taken, nor be modified during the lifetime of + // this `Font`. + pub struct Font { + #[cfg(feature = "swash")] + swash: (u32, swash::CacheKey), + rustybuzz: rustybuzz::Face<'static>, + // Note: This field must be after rustybuzz to ensure that it is dropped later. Otherwise + // there would be a dangling reference when dropping rustybuzz. + data: aliasable::boxed::AliasableBox + Send + Sync>>, + id: fontdb::ID, + } + + pub(super) struct FontTryBuilder< + RustybuzzBuilder: for<'this> FnOnce( + &'this Arc + Send + Sync>, + ) -> Option>, + > { + pub(super) id: fontdb::ID, + pub(super) data: Arc + Send + Sync>, + pub(super) rustybuzz_builder: RustybuzzBuilder, + #[cfg(feature = "swash")] + pub(super) swash: (u32, swash::CacheKey), + } + impl< + RustybuzzBuilder: for<'this> FnOnce( + &'this Arc + Send + Sync>, + ) -> Option>, + > FontTryBuilder + { + pub(super) fn try_build(self) -> Option { + unsafe fn change_lifetime<'old, 'new: 'old, T: 'new>(data: &'old T) -> &'new T { + &*(data as *const _) + } + + let data: AliasableBox + Send + Sync>> = + AliasableBox::from_unique(Box::new(self.data)); + + // Safety: We use AliasableBox to allow the references in rustybuzz::Face to alias with + // the data stored behind the AliasableBox. In addition the entire public interface of + // Font ensures that no mutable reference is given to data. And finally we use + // for<'this> for the rustybuzz_builder to ensure it can't leak a reference. Combined + // this ensures that it is sound to produce a self-referential type. + let rustybuzz = (self.rustybuzz_builder)(unsafe { change_lifetime(&*data) })?; + + Some(Font { + id: self.id, + data, + rustybuzz, + #[cfg(feature = "swash")] + swash: self.swash, + }) + } + } + + impl Font { + pub fn id(&self) -> fontdb::ID { + self.id + } + + pub fn data(&self) -> &[u8] { + // Safety: This only gives an immutable access to `data`. + (**self.data).as_ref() + } + + pub fn rustybuzz(&self) -> &rustybuzz::Face<'_> { + &self.rustybuzz + } + + #[cfg(feature = "swash")] + pub fn as_swash(&self) -> swash::FontRef<'_> { + let swash = &self.swash; + swash::FontRef { + data: self.data(), + offset: swash.0, + key: swash.1, + } + } + } } -#[cfg(feature = "swash")] -pub type SwashKey = (u32, swash::CacheKey); - -#[cfg(not(feature = "swash"))] -pub type SwashKey = (); - impl Font { pub fn new(info: &fontdb::FaceInfo) -> Option { #[allow(unused_variables)] @@ -40,56 +112,16 @@ impl Font { #[cfg(feature = "std")] fontdb::Source::SharedFile(_path, data) => Arc::clone(data), }; - Some(Self( - FontInnerTryBuilder { - id: info.id, - swash: { - #[cfg(feature = "swash")] - let swash = { - let swash = - swash::FontRef::from_index((*data).as_ref(), info.index as usize)?; - (swash.offset, swash.key) - }; - #[cfg(not(feature = "swash"))] - let swash = (); - swash - }, - data, - rustybuzz_builder: |data| { - rustybuzz::Face::from_slice((**data).as_ref(), info.index).ok_or(()) - }, - } - .try_build() - .ok()?, - )) - } - - pub fn id(&self) -> fontdb::ID { - *self.0.borrow_id() - } - - pub fn data(&self) -> &[u8] { - (**self.0.borrow_data()).as_ref() - } - - pub fn rustybuzz(&self) -> &rustybuzz::Face { - self.0.borrow_rustybuzz() - } - - #[cfg(feature = "swash")] - pub fn as_swash(&self) -> swash::FontRef { - let swash = self.0.borrow_swash(); - swash::FontRef { - data: self.data(), - offset: swash.0, - key: swash.1, + font_inner::FontTryBuilder { + id: info.id, + #[cfg(feature = "swash")] + swash: { + let swash = swash::FontRef::from_index((*data).as_ref(), info.index as usize)?; + (swash.offset, swash.key) + }, + data, + rustybuzz_builder: |data| rustybuzz::Face::from_slice((**data).as_ref(), info.index), } - } - - // This is used to prevent warnings due to the swash field being unused. - #[cfg(not(feature = "swash"))] - #[allow(dead_code)] - fn as_swash(&self) { - self.0.borrow_swash(); + .try_build() } }