From de9fb7983aa72da9b531bbe3d004e0b859615d47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 14 Jun 2023 21:08:22 +0000 Subject: [PATCH] style: Speed up / specialize attribute selector-matching Inline the attribute lookup code, and only branch on the attribute selector type if we have found an attribute. Differential Revision: https://phabricator.services.mozilla.com/D180531 --- components/style/gecko/snapshot.rs | 69 +-------- components/style/gecko/snapshot_helpers.rs | 121 +++++++++++++++- components/style/gecko/wrapper.rs | 160 +++++++-------------- 3 files changed, 170 insertions(+), 180 deletions(-) diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 8adde0a927b..372e7fdb7f5 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -7,7 +7,7 @@ use crate::dom::TElement; use crate::gecko::snapshot_helpers; -use crate::gecko::wrapper::{GeckoElement, NamespaceConstraintHelpers}; +use crate::gecko::wrapper::GeckoElement; use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs::ServoElementSnapshot; use crate::gecko_bindings::structs::ServoElementSnapshotFlags as Flags; @@ -19,8 +19,7 @@ use crate::values::{AtomIdent, AtomString}; use crate::LocalName; use crate::WeakAtom; use dom::ElementState; -use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator}; -use selectors::attr::{CaseSensitivity, NamespaceConstraint}; +use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint}; /// A snapshot of a Gecko element. pub type GeckoElementSnapshot = ServoElementSnapshot; @@ -91,69 +90,7 @@ impl GeckoElementSnapshot { local_name: &LocalName, operation: &AttrSelectorOperation<&AttrValue>, ) -> bool { - unsafe { - match *operation { - AttrSelectorOperation::Exists => { - bindings::Gecko_SnapshotHasAttr(self, ns.atom_or_null(), local_name.as_ptr()) - }, - AttrSelectorOperation::WithValue { - operator, - case_sensitivity, - value, - } => { - let ignore_case = match case_sensitivity { - CaseSensitivity::CaseSensitive => false, - CaseSensitivity::AsciiCaseInsensitive => true, - }; - match operator { - AttrSelectorOperator::Equal => bindings::Gecko_SnapshotAttrEquals( - self, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Includes => bindings::Gecko_SnapshotAttrIncludes( - self, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::DashMatch => bindings::Gecko_SnapshotAttrDashEquals( - self, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Prefix => bindings::Gecko_SnapshotAttrHasPrefix( - self, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Suffix => bindings::Gecko_SnapshotAttrHasSuffix( - self, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Substring => { - bindings::Gecko_SnapshotAttrHasSubstring( - self, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ) - }, - } - }, - } - } + snapshot_helpers::attr_matches(self.mAttrs.iter(), ns, local_name, operation) } } diff --git a/components/style/gecko/snapshot_helpers.rs b/components/style/gecko/snapshot_helpers.rs index 73d740012b2..818e767e0e9 100644 --- a/components/style/gecko/snapshot_helpers.rs +++ b/components/style/gecko/snapshot_helpers.rs @@ -5,15 +5,15 @@ //! Element an snapshot common logic. use crate::dom::TElement; +use crate::gecko::wrapper::namespace_id_to_atom; use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs::{self, nsAtom}; use crate::invalidation::element::element_wrapper::ElementSnapshot; -use crate::selector_parser::SnapshotMap; +use crate::selector_parser::{AttrValue, SnapshotMap}; use crate::string_cache::WeakAtom; use crate::values::AtomIdent; -use crate::Atom; -use crate::CaseSensitivityExt; -use selectors::attr::CaseSensitivity; +use crate::{Atom, CaseSensitivityExt, LocalName, Namespace}; +use selectors::attr::{CaseSensitivity, NamespaceConstraint, AttrSelectorOperation, AttrSelectorOperator}; use smallvec::SmallVec; /// A function that, given an element of type `T`, allows you to get a single @@ -72,6 +72,37 @@ unsafe fn get_id_from_attr(attr: &structs::nsAttrValue) -> &WeakAtom { WeakAtom::new(ptr::(attr)) } +impl structs::nsAttrName { + #[inline] + fn is_nodeinfo(&self) -> bool { + self.mBits & 1 != 0 + } + + #[inline] + unsafe fn as_nodeinfo(&self) -> &structs::NodeInfo { + debug_assert!(self.is_nodeinfo()); + &*((self.mBits & !1) as *const structs::NodeInfo) + } + + #[inline] + fn namespace_id(&self) -> i32 { + if !self.is_nodeinfo() { + return structs::kNameSpaceID_None; + } + unsafe { self.as_nodeinfo() }.mInner.mNamespaceID + } + + /// Returns the attribute name as an atom pointer. + #[inline] + pub fn name(&self) -> *const nsAtom { + if self.is_nodeinfo() { + unsafe { self.as_nodeinfo() }.mInner.mName + } else { + self.mBits as *const nsAtom + } + } +} + /// Find an attribute value with a given name and no namespace. #[inline(always)] pub fn find_attr<'a>( @@ -194,3 +225,85 @@ pub fn classes_changed(element: &E, snapshots: &SnapshotMap) -> Sma classes_changed } + +/// Returns whether a given attribute selector matches given the internal attrs. +pub(crate) fn attr_matches<'a>( + iter: impl Iterator, + ns: &NamespaceConstraint<&Namespace>, + local_name: &LocalName, + operation: &AttrSelectorOperation<&AttrValue>, +) -> bool { + let name_ptr = local_name.as_ptr(); + for attr in iter { + if attr.mName.name() != name_ptr { + continue; + } + + let ns_matches = match *ns { + NamespaceConstraint::Any => true, + NamespaceConstraint::Specific(ns) => { + if *ns == ns!() { + !attr.mName.is_nodeinfo() + } else { + ns.as_ptr() == unsafe { namespace_id_to_atom(attr.mName.namespace_id()) } + } + }, + }; + + if !ns_matches { + continue; + } + + let (operator, case_sensitivity, value) = match *operation { + AttrSelectorOperation::Exists => return true, + AttrSelectorOperation::WithValue { + operator, + case_sensitivity, + value, + } => (operator, case_sensitivity, value), + }; + let ignore_case = match case_sensitivity { + CaseSensitivity::CaseSensitive => false, + CaseSensitivity::AsciiCaseInsensitive => true, + }; + let value = value.as_ptr(); + let matches = unsafe { + match operator { + AttrSelectorOperator::Equal => bindings::Gecko_AttrEquals( + &attr.mValue, + value, + ignore_case, + ), + AttrSelectorOperator::Includes => bindings::Gecko_AttrIncludes( + &attr.mValue, + value, + ignore_case, + ), + AttrSelectorOperator::DashMatch => bindings::Gecko_AttrDashEquals( + &attr.mValue, + value, + ignore_case, + ), + AttrSelectorOperator::Prefix => bindings::Gecko_AttrHasPrefix( + &attr.mValue, + value, + ignore_case, + ), + AttrSelectorOperator::Suffix => bindings::Gecko_AttrHasSuffix( + &attr.mValue, + value, + ignore_case, + ), + AttrSelectorOperator::Substring => bindings::Gecko_AttrHasSubstring( + &attr.mValue, + value, + ignore_case, + ), + } + }; + if matches || *ns != NamespaceConstraint::Any { + return matches; + } + } + false +} diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 98b5de4f44c..490c063fbe7 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -68,8 +68,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use dom::{DocumentState, ElementState}; use euclid::default::Size2D; use fxhash::FxHashMap; -use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator}; -use selectors::attr::{CaseSensitivity, NamespaceConstraint}; +use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint}; use selectors::matching::VisitedHandlingMode; use selectors::matching::{ElementSelectorFlags, MatchingContext}; use selectors::sink::Push; @@ -589,10 +588,30 @@ impl<'le> GeckoElement<'le> { self.may_have_animations() && unsafe { Gecko_ElementHasAnimations(self.0) } } + #[inline(always)] + fn attrs(&self) -> Option<&structs::AttrArray_Impl> { + unsafe { self.0.mAttrs.mImpl.mPtr.as_ref() } + } + #[inline(always)] fn non_mapped_attrs(&self) -> &[structs::AttrArray_InternalAttr] { + let attrs = match self.attrs() { + Some(attrs) => attrs, + None => return &[], + }; unsafe { - let attrs = match self.0.mAttrs.mImpl.mPtr.as_ref() { + attrs.mBuffer.as_slice(attrs.mAttrCount as usize) + } + } + + #[inline(always)] + fn mapped_attrs(&self) -> &[structs::AttrArray_InternalAttr] { + let attrs = match self.attrs() { + Some(attrs) => attrs, + None => return &[], + }; + unsafe { + let attrs = match attrs.mMappedAttrs.as_ref() { Some(attrs) => attrs, None => return &[], }; @@ -601,21 +620,9 @@ impl<'le> GeckoElement<'le> { } } - #[inline(always)] - fn mapped_attrs(&self) -> &[structs::AttrArray_InternalAttr] { - unsafe { - let attrs = match self.0.mAttrs.mImpl.mPtr.as_ref() { - Some(attrs) => attrs, - None => return &[], - }; - - let attrs = match attrs.mMappedAttrs.as_ref() { - Some(attrs) => attrs, - None => return &[], - }; - - attrs.mBuffer.as_slice(attrs.mAttrCount as usize) - } + #[inline] + fn iter_attrs(&self) -> impl Iterator { + self.non_mapped_attrs().iter().chain(self.mapped_attrs().iter()) } #[inline(always)] @@ -924,6 +931,16 @@ fn get_animation_rule( } } +/// Turns a gecko namespace id into an atom. Might panic if you pass any random thing that isn't a +/// namespace id. +#[inline(always)] +pub unsafe fn namespace_id_to_atom(id: i32) -> *mut nsAtom { + unsafe { + let namespace_manager = structs::nsNameSpaceManager_sInstance.mRawPtr; + (*namespace_manager).mURIArray[id as usize].mRawPtr + } +} + impl<'le> TElement for GeckoElement<'le> { type ConcreteNode = GeckoNode<'le>; type TraversalChildrenIterator = GeckoChildrenIterator<'le>; @@ -1004,10 +1021,7 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn namespace(&self) -> &WeakNamespace { - unsafe { - let namespace_manager = structs::nsNameSpaceManager_sInstance.mRawPtr; - WeakNamespace::new((*namespace_manager).mURIArray[self.namespace_id() as usize].mRawPtr) - } + unsafe { WeakNamespace::new(namespace_id_to_atom(self.namespace_id())) } } #[inline] @@ -1201,20 +1215,9 @@ impl<'le> TElement for GeckoElement<'le> { where F: FnMut(&AtomIdent), { - for attr in self - .non_mapped_attrs() - .iter() - .chain(self.mapped_attrs().iter()) - { - let is_nodeinfo = attr.mName.mBits & 1 != 0; + for attr in self.iter_attrs() { unsafe { - let atom = if is_nodeinfo { - let node_info = &*((attr.mName.mBits & !1) as *const structs::NodeInfo); - node_info.mInner.mName - } else { - attr.mName.mBits as *const nsAtom - }; - AtomIdent::with(atom, |a| callback(a)) + AtomIdent::with(attr.mName.name(), |a| callback(a)) } } } @@ -1847,73 +1850,25 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } } + fn has_attr_in_no_namespace(&self, local_name: &LocalName) -> bool { + for attr in self.iter_attrs() { + if attr.mName.mBits == local_name.as_ptr() as usize { + return true; + } + } + false + } + fn attr_matches( &self, ns: &NamespaceConstraint<&Namespace>, local_name: &LocalName, operation: &AttrSelectorOperation<&AttrValue>, ) -> bool { - unsafe { - match *operation { - AttrSelectorOperation::Exists => { - bindings::Gecko_HasAttr(self.0, ns.atom_or_null(), local_name.as_ptr()) - }, - AttrSelectorOperation::WithValue { - operator, - case_sensitivity, - value, - } => { - let ignore_case = match case_sensitivity { - CaseSensitivity::CaseSensitive => false, - CaseSensitivity::AsciiCaseInsensitive => true, - }; - match operator { - AttrSelectorOperator::Equal => bindings::Gecko_AttrEquals( - self.0, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Includes => bindings::Gecko_AttrIncludes( - self.0, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::DashMatch => bindings::Gecko_AttrDashEquals( - self.0, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Prefix => bindings::Gecko_AttrHasPrefix( - self.0, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Suffix => bindings::Gecko_AttrHasSuffix( - self.0, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - AttrSelectorOperator::Substring => bindings::Gecko_AttrHasSubstring( - self.0, - ns.atom_or_null(), - local_name.as_ptr(), - value.as_ptr(), - ignore_case, - ), - } - }, - } + if self.attrs().is_none() { + return false; } + snapshot_helpers::attr_matches(self.iter_attrs(), ns, local_name, operation) } #[inline] @@ -2168,18 +2123,3 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { self.is_root_of_native_anonymous_subtree() } } - -/// A few helpers to help with attribute selectors and snapshotting. -pub trait NamespaceConstraintHelpers { - /// Returns the namespace of the selector, or null otherwise. - fn atom_or_null(&self) -> *mut nsAtom; -} - -impl<'a> NamespaceConstraintHelpers for NamespaceConstraint<&'a Namespace> { - fn atom_or_null(&self) -> *mut nsAtom { - match *self { - NamespaceConstraint::Any => ptr::null_mut(), - NamespaceConstraint::Specific(ref ns) => ns.0.as_ptr(), - } - } -}