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

@@ -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::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
-use selectors::attr::{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 {
+        snapshot_helpers::attr_matches(self.mAttrs.iter(), ns, local_name, operation)
-            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,
-                            )
-                        },
-                    }
-                },
-            }
-        }
     }
 }
 

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::{Atom, CaseSensitivityExt, LocalName, Namespace};
-use crate::CaseSensitivityExt;
+use selectors::attr::{CaseSensitivity, NamespaceConstraint, AttrSelectorOperation, AttrSelectorOperator};
-use selectors::attr::CaseSensitivity;
 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::<nsAtom>(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<E: TElement>(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<Item = &'a structs::AttrArray_InternalAttr>,
+    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
+}

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::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
-use selectors::attr::{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)]
+    #[inline]
-    fn mapped_attrs(&self) -> &[structs::AttrArray_InternalAttr] {
+    fn iter_attrs(&self) -> impl Iterator<Item = &structs::AttrArray_InternalAttr> {
-        unsafe {
+        self.non_mapped_attrs().iter().chain(self.mapped_attrs().iter())
-            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(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 {
+        unsafe { WeakNamespace::new(namespace_id_to_atom(self.namespace_id())) }
-            let namespace_manager = structs::nsNameSpaceManager_sInstance.mRawPtr;
-            WeakNamespace::new((*namespace_manager).mURIArray[self.namespace_id() as usize].mRawPtr)
-        }
     }
 
     #[inline]
@@ -1201,20 +1215,9 @@ impl<'le> TElement for GeckoElement<'le> {
     where
         F: FnMut(&AtomIdent),
     {
-        for attr in self
+        for attr in self.iter_attrs() {
-            .non_mapped_attrs()
-            .iter()
-            .chain(self.mapped_attrs().iter())
-        {
-            let is_nodeinfo = attr.mName.mBits & 1 != 0;
             unsafe {
-                let atom = if is_nodeinfo {
+                AtomIdent::with(attr.mName.name(), |a| callback(a))
-                    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))
             }
         }
     }
@@ -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 {
+        if self.attrs().is_none() {
-            match *operation {
+            return false;
-                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,
-                        ),
-                    }
-                },
-            }
         }
+        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(),
-        }
-    }
-}