Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-10 03:45:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1828316 - Avoid linear search to check for custom property presence. r=firefox-style-system-reviewers,zrhoffman

Browse Source 
The test-case in the next patch from 10s to a couple hundred
milliseconds on my machine. No behavior change other than that.

Differential Revision: https://phabricator.services.mozilla.com/D175612
This commit is contained in:
Emilio Cobos Álvarez 2023-04-18 11:09:31 +00:00
parent ab53a871b9
commit 26057c5cec
3 changed files with 112 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

173
servo/components/style/properties/declaration_block.rs
View File

@ -9,11 +9,12 @@
use super::*;
use crate::applicable_declarations::CascadePriority;
use crate::context::QuirksMode;
use crate::custom_properties::CustomPropertiesBuilder;
use crate::custom_properties::{self, CustomPropertiesBuilder};
use crate::error_reporting::{ContextualParseError, ParseErrorReporter};
use crate::parser::ParserContext;
use crate::properties::animated_properties::{AnimationValue, AnimationValueMap};
use crate::rule_tree::CascadeLevel;
use crate::selector_map::PrecomputedHashSet;
use crate::selector_parser::SelectorImpl;
use crate::shared_lock::Locked;
use crate::str::{CssString, CssStringWriter};
@ -29,6 +30,7 @@ use std::fmt::{self, Write};
use std::iter::{DoubleEndedIterator, Zip};
use std::slice::Iter;
use style_traits::{CssWriter, ParseError, ParsingMode, StyleParseErrorKind, ToCss};
use thin_vec::ThinVec;
/// A set of property declarations including animations and transitions.
#[derive(Default)]
@ -46,7 +48,6 @@ impl AnimationDeclarations {
}
}
/// An enum describes how a declaration should update
/// the declaration block.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@ -93,6 +94,48 @@ impl Importance {
}
}
#[derive(Clone, ToShmem, Default, MallocSizeOf)]
struct PropertyDeclarationIdSet {
longhands: LonghandIdSet,
custom: PrecomputedHashSet<custom_properties::Name>,
}
impl PropertyDeclarationIdSet {
fn insert(&mut self, id: PropertyDeclarationId) -> bool {
match id {
PropertyDeclarationId::Longhand(id) => {
if self.longhands.contains(id) {
return false;
}
self.longhands.insert(id);
return true;
},
PropertyDeclarationId::Custom(name) => self.custom.insert(name.clone()),
}
}
fn contains(&self, id: PropertyDeclarationId) -> bool {
match id {
PropertyDeclarationId::Longhand(id) => self.longhands.contains(id),
PropertyDeclarationId::Custom(name) => self.custom.contains(name),
}
}
fn remove(&mut self, id: PropertyDeclarationId) {
match id {
PropertyDeclarationId::Longhand(id) => self.longhands.remove(id),
PropertyDeclarationId::Custom(name) => {
self.custom.remove(name);
},
}
}
fn clear(&mut self) {
self.longhands.clear();
self.custom.clear();
}
}
/// Overridden declarations are skipped.
#[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
#[derive(Clone, ToShmem)]
@ -100,12 +143,13 @@ pub struct PropertyDeclarationBlock {
/// The group of declarations, along with their importance.
///
/// Only deduplicated declarations appear here.
declarations: Vec<PropertyDeclaration>,
declarations: ThinVec<PropertyDeclaration>,
/// The "important" flag for each declaration in `declarations`.
declarations_importance: SmallBitVec,
longhands: LonghandIdSet,
/// The set of properties that are present in the block.
property_ids: PropertyDeclarationIdSet,
}
/// Iterator over `(PropertyDeclaration, Importance)` pairs.
@ -236,22 +280,22 @@ impl PropertyDeclarationBlock {
#[inline]
pub fn new() -> Self {
PropertyDeclarationBlock {
declarations: Vec::new(),
declarations: ThinVec::new(),
declarations_importance: SmallBitVec::new(),
longhands: LonghandIdSet::new(),
property_ids: PropertyDeclarationIdSet::default(),
}
}
/// Create a block with a single declaration
pub fn with_one(declaration: PropertyDeclaration, importance: Importance) -> Self {
let mut longhands = LonghandIdSet::new();
if let PropertyDeclarationId::Longhand(id) = declaration.id() {
longhands.insert(id);
}
let mut property_ids = PropertyDeclarationIdSet::default();
property_ids.insert(declaration.id());
let mut declarations = ThinVec::with_capacity(1);
declarations.push(declaration);
PropertyDeclarationBlock {
declarations: vec![declaration],
declarations,
declarations_importance: SmallBitVec::from_elem(1, importance.important()),
longhands,
property_ids,
}
}
@ -312,23 +356,23 @@ impl PropertyDeclarationBlock {
!self.declarations_importance.all_true()
}
/// Returns whether this block contains a declaration of a given longhand.
/// Returns a `LonghandIdSet` representing the properties that are changed in
/// this block.
#[inline]
pub fn contains(&self, id: LonghandId) -> bool {
self.longhands.contains(id)
pub fn longhands(&self) -> &LonghandIdSet {
&self.property_ids.longhands
}
/// Returns whether this block contains a declaration of a given property id.
#[inline]
pub fn contains(&self, id: PropertyDeclarationId) -> bool {
self.property_ids.contains(id)
}
/// Returns whether this block contains any reset longhand.
#[inline]
pub fn contains_any_reset(&self) -> bool {
self.longhands.contains_any_reset()
}
/// Returns a `LonghandIdSet` representing the properties that are changed in
/// this block.
#[inline]
pub fn longhands(&self) -> &LonghandIdSet {
&self.longhands
self.property_ids.longhands.contains_any_reset()
}
/// Get a declaration for a given property.
@ -340,12 +384,9 @@ impl PropertyDeclarationBlock {
&self,
property: PropertyDeclarationId,
) -> Option<(&PropertyDeclaration, Importance)> {
if let PropertyDeclarationId::Longhand(id) = property {
if !self.contains(id) {
return None;
}
if !self.contains(property) {
return None;
}
self.declaration_importance_iter()
.find(|(declaration, _)| declaration.id() == property)
}
@ -444,16 +485,6 @@ impl PropertyDeclarationBlock {
}
}
/// Returns whether the property is definitely new for this declaration
/// block. It returns true when the declaration is a non-custom longhand
/// and it doesn't exist in the block, and returns false otherwise.
#[inline]
fn is_definitely_new(&self, decl: &PropertyDeclaration) -> bool {
decl.id()
.as_longhand()
.map_or(false, |id| !self.longhands.contains(id))
}
/// Adds or overrides the declaration for a given property in this block.
///
/// See the documentation of `push` to see what impact `source` has when the
@ -492,10 +523,11 @@ impl PropertyDeclarationBlock {
///
/// This is only used for parsing and internal use.
pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) -> bool {
if !self.is_definitely_new(&declaration) {
let id = declaration.id();
if !self.property_ids.insert(id) {
let mut index_to_remove = None;
for (i, slot) in self.declarations.iter_mut().enumerate() {
if slot.id() != declaration.id() {
if slot.id() != id {
continue;
}
@ -520,9 +552,6 @@ impl PropertyDeclarationBlock {
}
}
if let PropertyDeclarationId::Longhand(id) = declaration.id() {
self.longhands.insert(id);
}
self.declarations.push(declaration);
self.declarations_importance.push(importance.important());
true
@ -545,7 +574,7 @@ impl PropertyDeclarationBlock {
.all_shorthand
.declarations()
.any(|decl| {
self.is_definitely_new(&decl) ||
!self.contains(decl.id()) ||
self.declarations
.iter()
.enumerate()
@ -566,7 +595,7 @@ impl PropertyDeclarationBlock {
.declarations
.iter()
.map(|declaration| {
if self.is_definitely_new(declaration) {
if !self.contains(declaration.id()) {
return DeclarationUpdate::Append;
}
let longhand_id = declaration.id().as_longhand();
@ -641,11 +670,10 @@ impl PropertyDeclarationBlock {
if !matches!(drain.all_shorthand, AllShorthand::NotSet) {
debug_assert!(updates.updates.is_empty());
for decl in drain.all_shorthand.declarations() {
if self.is_definitely_new(&decl) {
let longhand_id = decl.id().as_longhand().unwrap();
let id = decl.id();
if self.property_ids.insert(id) {
self.declarations.push(decl);
self.declarations_importance.push(important);
self.longhands.insert(longhand_id);
} else {
let (idx, slot) = self
.declarations
@ -712,9 +740,7 @@ impl PropertyDeclarationBlock {
match *update {
DeclarationUpdate::None => {},
DeclarationUpdate::Append | DeclarationUpdate::AppendAndRemove { .. } => {
if let Some(id) = decl.id().as_longhand() {
self.longhands.insert(id);
}
self.property_ids.insert(decl.id());
self.declarations.push(decl);
self.declarations_importance.push(important);
},
@ -731,8 +757,8 @@ impl PropertyDeclarationBlock {
/// `property`.
#[inline]
pub fn first_declaration_to_remove(&self, property: &PropertyId) -> Option<usize> {
if let Some(id) = property.longhand_id() {
if !self.longhands.contains(id) {
if let Err(longhand_or_custom) = property.as_shorthand() {
if !self.contains(longhand_or_custom) {
return None;
}
}
@ -745,13 +771,8 @@ impl PropertyDeclarationBlock {
/// Removes a given declaration at a given index.
#[inline]
fn remove_declaration_at(&mut self, i: usize) {
{
let id = self.declarations[i].id();
if let PropertyDeclarationId::Longhand(id) = id {
self.longhands.remove(id);
}
self.declarations_importance.remove(i);
}
self.property_ids.remove(self.declarations[i].id());
self.declarations_importance.remove(i);
self.declarations.remove(i);
}
@ -760,7 +781,7 @@ impl PropertyDeclarationBlock {
pub fn clear(&mut self) {
self.declarations_importance.clear();
self.declarations.clear();
self.longhands.clear();
self.property_ids.clear();
}
/// <https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-removeproperty>
@ -848,7 +869,7 @@ impl PropertyDeclarationBlock {
custom_properties.as_ref(),
QuirksMode::NoQuirks,
device,
&mut Default::default()
&mut Default::default(),
)
.to_css(dest)
},
@ -859,17 +880,17 @@ impl PropertyDeclarationBlock {
/// Convert AnimationValueMap to PropertyDeclarationBlock.
pub fn from_animation_value_map(animation_value_map: &AnimationValueMap) -> Self {
let len = animation_value_map.len();
let mut declarations = Vec::with_capacity(len);
let mut longhands = LonghandIdSet::new();
let mut declarations = ThinVec::with_capacity(len);
let mut property_ids = PropertyDeclarationIdSet::default();
for (property, animation_value) in animation_value_map.iter() {
longhands.insert(*property);
property_ids.longhands.insert(*property);
declarations.push(animation_value.uncompute());
}
PropertyDeclarationBlock {
declarations,
longhands,
property_ids,
declarations_importance: SmallBitVec::from_elem(len, false),
}
}
@ -877,8 +898,8 @@ impl PropertyDeclarationBlock {
/// Returns true if the declaration block has a CSSWideKeyword for the given
/// property.
pub fn has_css_wide_keyword(&self, property: &PropertyId) -> bool {
if let Some(id) = property.longhand_id() {
if !self.longhands.contains(id) {
if let Err(longhand_or_custom) = property.as_shorthand() {
if !self.property_ids.contains(longhand_or_custom) {
return false;
}
}
@ -997,7 +1018,7 @@ impl PropertyDeclarationBlock {
// If all properties that map to shorthand are not present
// in longhands, continue with the steps labeled shorthand
// loop.
if !self.longhands.contains_all(&longhands) {
if !self.property_ids.longhands.contains_all(&longhands) {
continue;
}
@ -1392,15 +1413,11 @@ fn report_one_css_error<'i>(
debug_assert!(context.error_reporting_enabled());
fn all_properties_in_block(block: &PropertyDeclarationBlock, property: &PropertyId) -> bool {
match *property {
PropertyId::LonghandAlias(id, _) | PropertyId::Longhand(id) => block.contains(id),
PropertyId::ShorthandAlias(id, _) | PropertyId::Shorthand(id) => {
id.longhands().all(|longhand| block.contains(longhand))
},
// NOTE(emilio): We could do this, but it seems of limited utility,
// and it's linear on the size of the declaration block, so let's
// not.
PropertyId::Custom(..) => false,
match property.as_shorthand() {
Ok(id) => id
.longhands()
.all(|longhand| block.contains(PropertyDeclarationId::Longhand(longhand))),
Err(longhand_or_custom) => block.contains(longhand_or_custom),
}
}

16
servo/components/to_shmem/lib.rs
View File

@ -24,7 +24,6 @@ use servo_arc::{Arc, ThinArc};
use smallbitvec::{InternalStorage, SmallBitVec};
use smallvec::{Array, SmallVec};
use std::alloc::Layout;
#[cfg(debug_assertions)]
use std::collections::HashSet;
use std::ffi::CString;
use std::isize;
@ -417,6 +416,21 @@ impl<T: ToShmem> ToShmem for Option<T> {
}
}
impl<T: ToShmem, S> ToShmem for HashSet<T, S>
where
Self: Default,
{
fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> Result<Self> {
if !self.is_empty() {
return Err(format!(
"ToShmem failed for HashSet: We only support empty sets \
(we don't expect custom properties in UA sheets, they're observable by content)",
))
}
Ok(ManuallyDrop::new(Self::default()))
}
}
impl<T: ToShmem> ToShmem for Arc<T> {
fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
// Assert that we don't encounter any shared references to values we

6
servo/ports/geckolib/glue.rs
View File

@ -5199,9 +5199,7 @@ macro_rules! get_longhand_from_id {
($id:expr) => {
match PropertyId::from_nscsspropertyid($id) {
Ok(PropertyId::Longhand(long)) => long,
_ => {
panic!("stylo: unknown presentation property with id");
},
_ => panic!("stylo: unknown presentation property with id"),
}
};
}
@ -5225,7 +5223,7 @@ pub extern "C" fn Servo_DeclarationBlock_PropertyIsSet(
property: nsCSSPropertyID,
) -> bool {
read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| {
decls.contains(get_longhand_from_id!(property))
decls.contains(PropertyDeclarationId::Longhand(get_longhand_from_id!(property)))
})
}