From 0fbb1aaaaa04d93ae478df6dc268ab8fb13e2bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20d=27Herbais=20de=20Thun?= Date: Mon, 27 Nov 2023 11:37:30 +0100 Subject: [PATCH] Optimize `Content::has`, `Introspector::query_label`, and `MetaElem` (#2759) --- crates/typst-macros/src/elem.rs | 37 +++++++++++++++++++ crates/typst/src/foundations/content.rs | 2 +- crates/typst/src/foundations/element.rs | 3 ++ .../typst/src/introspection/introspector.rs | 4 +- crates/typst/src/introspection/mod.rs | 5 ++- crates/typst/src/model/footnote.rs | 5 +-- crates/typst/src/model/reference.rs | 12 +++--- crates/typst/src/realize/mod.rs | 4 +- 8 files changed, 56 insertions(+), 16 deletions(-) diff --git a/crates/typst-macros/src/elem.rs b/crates/typst-macros/src/elem.rs index acb73ba54..e0306b706 100644 --- a/crates/typst-macros/src/elem.rs +++ b/crates/typst-macros/src/elem.rs @@ -738,6 +738,27 @@ fn create_native_elem_impl(element: &Elem) -> TokenStream { } }); + // Fields that can be checked using the `has` method. + let field_has_matches = element.visible_fields().map(|field| { + let elem = &element.ident; + let name = &field.enum_ident; + let field_ident = &field.ident; + + if field.ghost { + quote! { + <#elem as #foundations::ElementFields>::Fields::#name => false, + } + } else if field.inherent() { + quote! { + <#elem as #foundations::ElementFields>::Fields::#name => true, + } + } else { + quote! { + <#elem as #foundations::ElementFields>::Fields::#name => self.#field_ident.is_some(), + } + } + }); + // Fields that can be set using the `set_field` method. let field_set_matches = element .visible_fields() @@ -888,6 +909,10 @@ fn create_native_elem_impl(element: &Elem) -> TokenStream { }) .unwrap_or_else(|| quote! { None }); + let label_has_field = element + .unless_capability("Unlabellable", || quote! { self.label().is_some() }) + .unwrap_or_else(|| quote! { false }); + let mark_prepared = element .unless_capability("Unlabellable", || quote! { self.prepared = true; }) .unwrap_or_else(|| quote! {}); @@ -1026,6 +1051,18 @@ fn create_native_elem_impl(element: &Elem) -> TokenStream { } } + fn has(&self, id: u8) -> bool { + let Ok(id) = <#ident as #foundations::ElementFields>::Fields::try_from(id) else { + return false; + }; + + match id { + <#ident as #foundations::ElementFields>::Fields::Label => #label_has_field, + #(#field_has_matches)* + _ => false, + } + } + fn fields(&self) -> #foundations::Dict { let mut fields = #foundations::Dict::new(); #(#field_dict)* diff --git a/crates/typst/src/foundations/content.rs b/crates/typst/src/foundations/content.rs index 111b33ea6..fc9a49e21 100644 --- a/crates/typst/src/foundations/content.rs +++ b/crates/typst/src/foundations/content.rs @@ -535,7 +535,7 @@ impl Content { return false; }; - self.get(id).is_some() + self.0.has(id) } /// Access the specified field on the content. Returns the default value if diff --git a/crates/typst/src/foundations/element.rs b/crates/typst/src/foundations/element.rs index 8e4d159a0..64c92be97 100644 --- a/crates/typst/src/foundations/element.rs +++ b/crates/typst/src/foundations/element.rs @@ -265,6 +265,9 @@ pub trait NativeElement: Debug + Repr + Construct + Set + Send + Sync + 'static /// Get the field with the given field ID. fn field(&self, id: u8) -> Option; + /// Whether the element has the given field set. + fn has(&self, id: u8) -> bool; + /// Set the field with the given ID. fn set_field(&mut self, id: u8, value: Value) -> StrResult<()>; diff --git a/crates/typst/src/introspection/introspector.rs b/crates/typst/src/introspection/introspector.rs index 80374aca1..df454b849 100644 --- a/crates/typst/src/introspection/introspector.rs +++ b/crates/typst/src/introspection/introspector.rs @@ -195,13 +195,13 @@ impl Introspector { } /// Query for a unique element with the label. - pub fn query_label(&self, label: Label) -> StrResult> { + pub fn query_label(&self, label: Label) -> StrResult<&Prehashed> { let mut found = None; for elem in self.all().filter(|elem| elem.label() == Some(label)) { if found.is_some() { bail!("label `{}` occurs multiple times in the document", label.repr()); } - found = Some(elem.clone()); + found = Some(elem); } found.ok_or_else(|| { eco_format!("label `{}` does not exist in the document", label.repr()) diff --git a/crates/typst/src/introspection/mod.rs b/crates/typst/src/introspection/mod.rs index 49a1c53c2..0f5d11b14 100644 --- a/crates/typst/src/introspection/mod.rs +++ b/crates/typst/src/introspection/mod.rs @@ -27,6 +27,7 @@ use smallvec::SmallVec; use crate::foundations::{ cast, category, elem, ty, Behave, Behaviour, Category, Content, Repr, Scope, + Unlabellable, }; use crate::layout::PdfPageLabel; use crate::model::{Destination, Numbering}; @@ -53,7 +54,7 @@ pub fn define(global: &mut Scope) { } /// Hosts metadata and ensures metadata is produced even for empty elements. -#[elem(Behave)] +#[elem(Behave, Unlabellable)] pub struct MetaElem { /// Metadata that should be attached to all elements affected by this style /// property. @@ -61,6 +62,8 @@ pub struct MetaElem { pub data: SmallVec<[Meta; 1]>, } +impl Unlabellable for MetaElem {} + impl Behave for MetaElem { fn behaviour(&self) -> Behaviour { Behaviour::Invisible diff --git a/crates/typst/src/model/footnote.rs b/crates/typst/src/model/footnote.rs index a6e1e2ded..6a0a7c15b 100644 --- a/crates/typst/src/model/footnote.rs +++ b/crates/typst/src/model/footnote.rs @@ -1,8 +1,6 @@ use std::num::NonZeroUsize; use std::str::FromStr; -use comemo::Prehashed; - use crate::diag::{bail, At, SourceResult, StrResult}; use crate::engine::Engine; use crate::foundations::{ @@ -111,8 +109,7 @@ impl FootnoteElem { pub fn declaration_location(&self, engine: &Engine) -> StrResult { match self.body() { FootnoteBody::Reference(label) => { - let element: Prehashed = - engine.introspector.query_label(*label)?; + let element = engine.introspector.query_label(*label)?; let footnote = element .to::() .ok_or("referenced element should be a footnote")?; diff --git a/crates/typst/src/model/reference.rs b/crates/typst/src/model/reference.rs index d64de1ae6..6928896a3 100644 --- a/crates/typst/src/model/reference.rs +++ b/crates/typst/src/model/reference.rs @@ -148,7 +148,7 @@ impl Synthesize for RefElem { let target = *self.target(); if !BibliographyElem::has(engine, target) { - if let Ok(elem) = engine.introspector.query_label(target) { + if let Ok(elem) = engine.introspector.query_label(target).cloned() { self.push_element(Some(elem.into_inner())); return Ok(()); } @@ -180,6 +180,7 @@ impl Show for RefElem { return Ok(FootnoteElem::with_label(target).spanned(span).pack()); } + let elem = elem.clone(); let refable = elem .with::() .ok_or_else(|| { @@ -213,17 +214,16 @@ impl Show for RefElem { )) .at(span)?; + let loc = elem.location().unwrap(); let numbers = refable .counter() - .at(engine, elem.location().unwrap())? + .at(engine, loc)? .display(engine, &numbering.trimmed())?; let supplement = match self.supplement(styles).as_ref() { Smart::Auto => refable.supplement(), Smart::Custom(None) => Content::empty(), - Smart::Custom(Some(supplement)) => { - supplement.resolve(engine, [(*elem).clone()])? - } + Smart::Custom(Some(supplement)) => supplement.resolve(engine, [elem])?, }; let mut content = numbers; @@ -231,7 +231,7 @@ impl Show for RefElem { content = supplement + TextElem::packed("\u{a0}") + content; } - Ok(content.linked(Destination::Location(elem.location().unwrap()))) + Ok(content.linked(Destination::Location(loc))) })) } } diff --git a/crates/typst/src/realize/mod.rs b/crates/typst/src/realize/mod.rs index 81dbdb667..eda43b4db 100644 --- a/crates/typst/src/realize/mod.rs +++ b/crates/typst/src/realize/mod.rs @@ -134,7 +134,7 @@ pub fn realize( let span = elem.span(); let meta = Meta::Elem(elem.clone()); return Ok(Some( - (elem + MetaElem::new().pack().spanned(span)) + (elem + MetaElem::new().spanned(span).pack()) .styled(MetaElem::set_data(smallvec![meta])), )); } @@ -758,6 +758,6 @@ impl<'a> CiteGroupBuilder<'a> { fn finish(self) -> (Content, StyleChain<'a>) { let span = self.items.first().map(|cite| cite.span()).unwrap_or(Span::detached()); - (CiteGroup::new(self.items).pack().spanned(span), self.styles) + (CiteGroup::new(self.items).spanned(span).pack(), self.styles) } }