perf: Do not do binary search on attributes

Stranger6667 · Stranger6667 · commit 06a7865a78bb · 2024-01-16T10:43:27.000+01:00
diff --git a/bindings/javascript/__test__/index.spec.ts b/bindings/javascript/__test__/index.spec.ts
@@ -64,7 +64,7 @@ test("invalid style", (t) => {
 
 const inlinedHtml = `<html><head>
 
-<link href="/rss.xml" rel="alternate" title="RSS" type="application/rss+xml">
+<link rel="alternate" type="application/rss+xml" title="RSS" href="/rss.xml">
 
 </head>
 <body>
diff --git a/bindings/python/tests-py/test_inlining.py b/bindings/python/tests-py/test_inlining.py
@@ -118,7 +118,7 @@ def test_remote_stylesheet(href, kwargs):
         inlined
         == """<html><head>
 
-<link href="/rss.xml" rel="alternate" title="RSS" type="application/rss+xml">
+<link rel="alternate" type="application/rss+xml" title="RSS" href="/rss.xml">
 
 </head>
 <body>
diff --git a/css-inline/src/html/attributes.rs b/css-inline/src/html/attributes.rs
@@ -160,30 +160,27 @@ impl Attributes {
             let attr = attributes.swap_remove(idx);
             class = Some(Class::new(attr.value));
         }
-        attributes.sort_unstable_by(|a, b| a.name.cmp(&b.name));
         Attributes { attributes, class }
     }
 
-    pub(crate) fn find(&self, needle: &QualName) -> Option<usize> {
-        if let Ok(idx) = self
-            .attributes
-            .binary_search_by(|probe| probe.name.cmp(needle))
-        {
-            Some(idx)
-        } else {
-            None
-        }
+    pub(crate) fn find(&self, needle: &QualName) -> Option<&str> {
+        self.attributes.iter().find_map(|probe| {
+            if probe.name == *needle {
+                Some(&*probe.value)
+            } else {
+                None
+            }
+        })
     }
 
     /// Checks if the attributes map contains a given local name.
     pub(crate) fn contains(&self, local: html5ever::LocalName) -> bool {
-        let needle = QualName::new(None, ns!(), local);
-        self.find(&needle).is_some()
+        self.get(local).is_some()
     }
 
     /// Get the value of the attribute with the given local name, if it exists.
     pub(crate) fn get(&self, local: html5ever::LocalName) -> Option<&str> {
         let needle = QualName::new(None, ns!(), local);
-        self.find(&needle).map(|idx| &*self.attributes[idx].value)
+        self.find(&needle)
     }
 }
diff --git a/css-inline/src/html/element.rs b/css-inline/src/html/element.rs
@@ -163,7 +163,6 @@ impl<'a> selectors::Element for Element<'a> {
                     ns_url.clone(),
                     local_name.clone().into_inner(),
                 ))
-                .map(|idx| &*attrs.attributes[idx].value)
                 .map_or(false, |value| operation.eval_str(value)),
         }
     }
diff --git a/css-inline/src/html/parser.rs b/css-inline/src/html/parser.rs
@@ -231,11 +231,12 @@ impl TreeSink for Sink {
             .expect("not an element");
         let attributes = &mut element.attributes;
         for attr in attrs {
-            if let Err(idx) = attributes
+            if attributes
                 .attributes
-                .binary_search_by(|entry| entry.name.cmp(&attr.name))
+                .iter()
+                .any(|entry| entry.name == attr.name)
             {
-                attributes.attributes.insert(idx, attr);
+                attributes.attributes.push(attr);
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -163,7 +163,6 @@ impl<'a> selectors::Element for Element<'a> {`
`163`	`163`	`ns_url.clone(),`
`164`	`164`	`local_name.clone().into_inner(),`
`165`	`165`	`))`
`166`		`- .map(\|idx\| &*attrs.attributes[idx].value)`
`167`	`166`	`.map_or(false, \|value\| operation.eval_str(value)),`
`168`	`167`	`}`
`169`	`168`	`}`
Original file line number	Diff line number	Diff line change
`@@ -231,11 +231,12 @@ impl TreeSink for Sink {`
`231`	`231`	`.expect("not an element");`
`232`	`232`	`let attributes = &mut element.attributes;`
`233`	`233`	`for attr in attrs {`
`234`		`- if let Err(idx) = attributes`
	`234`	`+ if attributes`
`235`	`235`	`.attributes`
`236`		`- .binary_search_by(\|entry\| entry.name.cmp(&attr.name))`
	`236`	`+ .iter()`
	`237`	`+ .any(\|entry\| entry.name == attr.name)`
`237`	`238`	`{`
`238`		`- attributes.attributes.insert(idx, attr);`
	`239`	`+ attributes.attributes.push(attr);`
`239`	`240`	`}`
`240`	`241`	`}`
`241`	`242`	`}`