Commit 210b1fa8 authored by Emilio Cobos Álvarez's avatar Emilio Cobos Álvarez
Browse files

Bug 1704551 - Add attribute names to the bloom filter. r=boris 

Safari does this. This reduces the runtime in the example linked from
comment 0 quite a lot (40ms on a local opt build, from ~130ms on a
release nightly build).

I added a pref because there's a slight chance of performance
regressions on pages that do not use attribute selectors, as we're now
doing more unconditional work per element (adding the attributes to the
bloom filter). But the trade-off should be worth it, I think.

Differential Revision: https://phabricator.services.mozilla.com/D111689
parent 2d921c7e

modules/libpref/init/StaticPrefList.yaml

+8 −0
Original line number Diff line number Diff line
@@ -6502,6 +6502,14 @@ 
  value: true

  mirror: always



# Whether the bloom filter optimization is applied to attribute names too, not

# only classes / id / namespaces / etc.

- name: layout.css.bloom-filter-attribute-names.enabled

  type: RelaxedAtomicBool

  value: true

  mirror: always

  rust: true



# Set the number of device pixels per CSS pixel. A value <= 0 means choose

# automatically based on user settings for the platform (e.g., "UI scale factor"

# on Mac). A positive value is used as-is. This effectively controls the size

servo/components/selectors/parser.rs

+26 −0
Original line number Diff line number Diff line
@@ -209,6 +209,12 @@ macro_rules! with_all_bounds { 


            /// pseudo-elements

            type PseudoElement: $($CommonBounds)* + PseudoElement<Impl = Self>;



            /// Whether attribute hashes should be collected for filtering

            /// purposes.

            fn should_collect_attr_hash(_name: &Self::LocalName) -> bool {

                false

            }

        }

    }

}
@@ -475,6 +481,26 @@ where 
            Component::Class(ref class) if quirks_mode != QuirksMode::Quirks => {

                class.precomputed_hash()

            },

            Component::AttributeInNoNamespace { ref local_name, .. } if Impl::should_collect_attr_hash(local_name) => {

                // AttributeInNoNamespace is only used when local_name ==

                // local_name_lower.

                local_name.precomputed_hash()

            },

            Component::AttributeInNoNamespaceExists { ref local_name, ref local_name_lower, .. } => {

                // Only insert the local-name into the filter if it's all

                // lowercase.  Otherwise we would need to test both hashes, and

                // our data structures aren't really set up for that.

                if local_name != local_name_lower || !Impl::should_collect_attr_hash(local_name) {

                    continue;

                }

                local_name.precomputed_hash()

            },

            Component::AttributeOther(ref selector) => {

                if selector.local_name != selector.local_name_lower || !Impl::should_collect_attr_hash(&selector.local_name) {

                    continue;

                }

                selector.local_name.precomputed_hash()

            },

            Component::Is(ref list) | Component::Where(ref list) => {

                // :where and :is OR their selectors, so we can't put any hash

                // in the filter if there's more than one selector, as that'd

servo/components/style/bloom.rs

+17 −0
Original line number Diff line number Diff line
@@ -102,6 +102,15 @@ impl<E: TElement> PushedElement<E> { 
    }

}



/// Returns whether the attribute name is excluded from the bloom filter.

///

/// We do this for attributes that are very common but not commonly used in

/// selectors.

#[inline]

pub fn is_attr_name_excluded_from_filter(atom: &crate::Atom) -> bool {

    *atom == atom!("class") || *atom == atom!("id") || *atom == atom!("style")

}



fn each_relevant_element_hash<E, F>(element: E, mut f: F)

where

    E: TElement,
@@ -115,6 +124,14 @@ where 
    }



    element.each_class(|class| f(class.get_hash()));



    if static_prefs::pref!("layout.css.bloom-filter-attribute-names.enabled") {

        element.each_attr_name(|name| {

            if !is_attr_name_excluded_from_filter(name) {

                f(name.get_hash())

            }

        });

    }

}



impl<E: TElement> Drop for StyleBloom<E> {

servo/components/style/dom.rs

+5 −0
Original line number Diff line number Diff line
@@ -526,6 +526,11 @@ pub trait TElement: 
    {

    }



    /// Internal iterator for the attribute names of this element.

    fn each_attr_name<F>(&self, callback: F)

    where

        F: FnMut(&AtomIdent);



    /// Internal iterator for the part names that this element exports for a

    /// given part name.

    fn each_exported_part<F>(&self, _name: &AtomIdent, _callback: F)

servo/components/style/gecko/selector_parser.rs

+5 −0
Original line number Diff line number Diff line
@@ -247,6 +247,11 @@ impl ::selectors::SelectorImpl for SelectorImpl { 


    type PseudoElement = PseudoElement;

    type NonTSPseudoClass = NonTSPseudoClass;



    fn should_collect_attr_hash(name: &AtomIdent) -> bool {

        static_prefs::pref!("layout.css.bloom-filter-attribute-names.enabled") &&

            !crate::bloom::is_attr_name_excluded_from_filter(name)

    }

}



impl<'a> SelectorParser<'a> {