diff --git a/crates/typst-cli/src/fonts.rs b/crates/typst-cli/src/fonts.rs index f4711b826..7793840c0 100644 --- a/crates/typst-cli/src/fonts.rs +++ b/crates/typst-cli/src/fonts.rs @@ -1,6 +1,6 @@ -use std::cell::OnceCell; use std::fs; use std::path::PathBuf; +use std::sync::OnceLock; use fontdb::{Database, Source}; use typst::diag::StrResult; @@ -42,7 +42,7 @@ pub struct FontSlot { /// to a collection. index: u32, /// The lazily loaded font. - font: OnceCell>, + font: OnceLock>, } impl FontSlot { @@ -92,7 +92,7 @@ impl FontSearcher { self.fonts.push(FontSlot { path: path.clone(), index: face.index, - font: OnceCell::new(), + font: OnceLock::new(), }); } } @@ -112,7 +112,7 @@ impl FontSearcher { self.fonts.push(FontSlot { path: PathBuf::new(), index: i as u32, - font: OnceCell::from(Some(font)), + font: OnceLock::from(Some(font)), }); } }; diff --git a/crates/typst-cli/src/world.rs b/crates/typst-cli/src/world.rs index 32a495e23..8f8a7e5c0 100644 --- a/crates/typst-cli/src/world.rs +++ b/crates/typst-cli/src/world.rs @@ -1,6 +1,6 @@ -use std::cell::{OnceCell, RefCell, RefMut}; use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::sync::{OnceLock, RwLock}; use std::{fs, mem}; use chrono::{DateTime, Datelike, Local}; @@ -34,10 +34,10 @@ pub struct SystemWorld { /// Locations of and storage for lazily loaded fonts. fonts: Vec, /// Maps file ids to source files and buffers. - slots: RefCell>, + slots: RwLock>, /// The current datetime if requested. This is stored here to ensure it is /// always the same within one compilation. Reset between compilations. - now: OnceCell>, + now: OnceLock>, /// The export cache, used for caching output files in `typst watch` /// sessions. export_cache: ExportCache, @@ -78,8 +78,8 @@ impl SystemWorld { library: Prehashed::new(Library::build()), book: Prehashed::new(searcher.book), fonts: searcher.fonts, - slots: RefCell::default(), - now: OnceCell::new(), + slots: RwLock::new(HashMap::new()), + now: OnceLock::new(), export_cache: ExportCache::new(), }) } @@ -103,6 +103,7 @@ impl SystemWorld { pub fn dependencies(&mut self) -> impl Iterator + '_ { self.slots .get_mut() + .unwrap() .values() .filter(|slot| slot.accessed()) .filter_map(|slot| system_path(&self.root, slot.id).ok()) @@ -110,7 +111,7 @@ impl SystemWorld { /// Reset the compilation state in preparation of a new compilation. pub fn reset(&mut self) { - for slot in self.slots.get_mut().values_mut() { + for slot in self.slots.get_mut().unwrap().values_mut() { slot.reset(); } self.now.take(); @@ -147,11 +148,11 @@ impl World for SystemWorld { } fn source(&self, id: FileId) -> FileResult { - self.slot(id)?.source(&self.root) + self.slot(id, |slot| slot.source(&self.root)) } fn file(&self, id: FileId) -> FileResult { - self.slot(id)?.file(&self.root) + self.slot(id, |slot| slot.file(&self.root)) } fn font(&self, index: usize) -> Option { @@ -176,11 +177,12 @@ impl World for SystemWorld { impl SystemWorld { /// Access the canonical slot for the given file id. - #[tracing::instrument(skip_all)] - fn slot(&self, id: FileId) -> FileResult> { - Ok(RefMut::map(self.slots.borrow_mut(), |slots| { - slots.entry(id).or_insert_with(|| FileSlot::new(id)) - })) + fn slot(&self, id: FileId, f: F) -> T + where + F: FnOnce(&mut FileSlot) -> T, + { + let mut map = self.slots.write().unwrap(); + f(map.entry(id).or_insert_with(|| FileSlot::new(id))) } } diff --git a/crates/typst/src/engine.rs b/crates/typst/src/engine.rs index 7d25bad30..85fa4301f 100644 --- a/crates/typst/src/engine.rs +++ b/crates/typst/src/engine.rs @@ -1,4 +1,4 @@ -use std::cell::Cell; +use std::sync::atomic::{AtomicUsize, Ordering}; use comemo::{Track, Tracked, TrackedMut, Validate}; @@ -24,7 +24,7 @@ pub struct Engine<'a> { } impl Engine<'_> { - /// Perform a fallible operation that does not immediately terminate further + /// Performs a fallible operation that does not immediately terminate further /// execution. Instead it produces a delayed error that is only promoted to /// a fatal one if it remains at the end of the introspection loop. pub fn delayed(&mut self, f: F) -> T @@ -44,7 +44,6 @@ impl Engine<'_> { /// The route the engine took during compilation. This is used to detect /// cyclic imports and too much nesting. -#[derive(Clone)] pub struct Route<'a> { // We need to override the constraint's lifetime here so that `Tracked` is // covariant over the constraint. If it becomes invariant, we're in for a @@ -63,7 +62,7 @@ pub struct Route<'a> { /// know the exact length (that would defeat the whole purpose because it /// would prevent cache reuse of some computation at different, /// non-exceeding depths). - upper: Cell, + upper: AtomicUsize, } /// The maximum nesting depths. They are different so that even if show rule and @@ -84,7 +83,12 @@ impl Route<'_> { impl<'a> Route<'a> { /// Create a new, empty route. pub fn root() -> Self { - Self { id: None, outer: None, len: 0, upper: Cell::new(0) } + Self { + id: None, + outer: None, + len: 0, + upper: AtomicUsize::new(0), + } } /// Extend the route with another segment with a default length of 1. @@ -93,7 +97,7 @@ impl<'a> Route<'a> { outer: Some(outer), id: None, len: 1, - upper: Cell::new(usize::MAX), + upper: AtomicUsize::new(usize::MAX), } } @@ -138,7 +142,10 @@ impl<'a> Route<'a> { /// Whether the route's depth is less than or equal to the given depth. pub fn within(&self, depth: usize) -> bool { - if self.upper.get().saturating_add(self.len) <= depth { + use Ordering::Relaxed; + + let upper = self.upper.load(Relaxed); + if upper.saturating_add(self.len) <= depth { return true; } @@ -146,8 +153,10 @@ impl<'a> Route<'a> { Some(_) if depth < self.len => false, Some(outer) => { let within = outer.within(depth - self.len); - if within && depth < self.upper.get() { - self.upper.set(depth); + if within && depth < upper { + // We don't want to accidentally increase the upper bound, + // hence the compare-exchange. + self.upper.compare_exchange(upper, depth, Relaxed, Relaxed).ok(); } within } @@ -161,3 +170,16 @@ impl Default for Route<'_> { Self::root() } } + +impl Clone for Route<'_> { + fn clone(&self) -> Self { + Self { + outer: self.outer, + id: self.id, + len: self.len, + // The ordering doesn't really matter since it's the upper bound + // is only an optimization. + upper: AtomicUsize::new(self.upper.load(Ordering::Relaxed)), + } + } +} diff --git a/tests/src/tests.rs b/tests/src/tests.rs index 465c5b6fd..f06783097 100644 --- a/tests/src/tests.rs +++ b/tests/src/tests.rs @@ -1,12 +1,12 @@ #![allow(clippy::comparison_chain)] -use std::cell::{RefCell, RefMut}; use std::collections::{HashMap, HashSet}; use std::ffi::OsStr; use std::fmt::{self, Display, Formatter, Write as _}; use std::io::{self, IsTerminal, Write}; use std::ops::Range; use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR}; +use std::sync::{OnceLock, RwLock}; use std::{env, fs}; use clap::Parser; @@ -14,7 +14,6 @@ use comemo::{Prehashed, Track}; use ecow::EcoString; use oxipng::{InFile, Options, OutFile}; use rayon::iter::{ParallelBridge, ParallelIterator}; -use std::cell::OnceCell; use tiny_skia as sk; use typst::diag::{bail, FileError, FileResult, Severity, StrResult}; use typst::eval::Tracer; @@ -217,20 +216,19 @@ fn library() -> Library { } /// A world that provides access to the tests environment. -#[derive(Clone)] struct TestWorld { print: PrintConfig, main: FileId, library: Prehashed, book: Prehashed, fonts: Vec, - paths: RefCell>, + slots: RwLock>, } #[derive(Clone)] -struct PathSlot { - source: OnceCell>, - buffer: OnceCell>, +struct FileSlot { + source: OnceLock>, + buffer: OnceLock>, } impl TestWorld { @@ -253,7 +251,7 @@ impl TestWorld { library: Prehashed::new(library()), book: Prehashed::new(FontBook::from_fonts(&fonts)), fonts, - paths: RefCell::default(), + slots: RwLock::new(HashMap::new()), } } } @@ -272,21 +270,23 @@ impl World for TestWorld { } fn source(&self, id: FileId) -> FileResult { - let slot = self.slot(id)?; - slot.source - .get_or_init(|| { - let buf = read(&system_path(id)?)?; - let text = String::from_utf8(buf)?; - Ok(Source::new(id, text)) - }) - .clone() + self.slot(id, |slot| { + slot.source + .get_or_init(|| { + let buf = read(&system_path(id)?)?; + let text = String::from_utf8(buf)?; + Ok(Source::new(id, text)) + }) + .clone() + }) } fn file(&self, id: FileId) -> FileResult { - let slot = self.slot(id)?; - slot.buffer - .get_or_init(|| read(&system_path(id)?).map(Bytes::from)) - .clone() + self.slot(id, |slot| { + slot.buffer + .get_or_init(|| read(&system_path(id)?).map(Bytes::from)) + .clone() + }) } fn font(&self, id: usize) -> Option { @@ -301,22 +301,37 @@ impl World for TestWorld { impl TestWorld { fn set(&mut self, path: &Path, text: String) -> Source { self.main = FileId::new(None, VirtualPath::new(path)); - let mut slot = self.slot(self.main).unwrap(); let source = Source::new(self.main, text); - slot.source = OnceCell::from(Ok(source.clone())); - source + self.slot(self.main, |slot| { + slot.source = OnceLock::from(Ok(source.clone())); + source + }) } - fn slot(&self, id: FileId) -> FileResult> { - Ok(RefMut::map(self.paths.borrow_mut(), |paths| { - paths.entry(id).or_insert_with(|| PathSlot { - source: OnceCell::new(), - buffer: OnceCell::new(), - }) + fn slot(&self, id: FileId, f: F) -> T + where + F: FnOnce(&mut FileSlot) -> T, + { + f(self.slots.write().unwrap().entry(id).or_insert_with(|| FileSlot { + source: OnceLock::new(), + buffer: OnceLock::new(), })) } } +impl Clone for TestWorld { + fn clone(&self) -> Self { + Self { + print: self.print, + main: self.main, + library: self.library.clone(), + book: self.book.clone(), + fonts: self.fonts.clone(), + slots: RwLock::new(self.slots.read().unwrap().clone()), + } + } +} + /// The file system path for a file ID. fn system_path(id: FileId) -> FileResult { let root: PathBuf = match id.package() {