style: Optimize serialization of identifiers of length <= 16 🐉🐲

Much like we optimize to_ascii_lowercase.

This also fixes a bug in Servo where attr() rules with an unknown namespace
prefix are parsed, which is wrong.

Cargo.lock

@@ -2918,6 +2918,7 @@ dependencies = [
  "servo_url 0.0.1",
  "smallbitvec 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "string_cache 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "style_derive 0.0.1",
  "style_traits 0.0.1",
  "time 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)",

components/style/Cargo.toml

@@ -21,7 +21,7 @@ gecko = ["nsstring", "num_cpus",
 use_bindgen = ["bindgen", "regex", "toml"]
 servo = ["serde", "style_traits/servo", "servo_atoms", "servo_config", "html5ever",
          "cssparser/serde", "encoding_rs", "malloc_size_of/servo", "arrayvec/use_union",
-         "servo_url"]
+         "servo_url", "string_cache"]
 gecko_debug = ["nsstring/gecko_debug"]
 
 [dependencies]
@@ -61,6 +61,7 @@ servo_atoms = {path = "../atoms", optional = true}
 servo_config = {path = "../config", optional = true}
 smallbitvec = "1.0.6"
 smallvec = "0.6"
+string_cache = { version = "0.7", optional = true }
 style_derive = {path = "../style_derive"}
 style_traits = {path = "../style_traits"}
 servo_url = {path = "../url", optional = true}

components/style/gecko/generated/pseudo_element_definition.rs

@@ -1728,10 +1728,10 @@ impl ToCss for PseudoElement {
                 dest.write_char('(')?;
                 let mut iter = args.iter();
                 if let Some(first) = iter.next() {
-                    serialize_identifier(&first.to_string(), dest)?;
+                    serialize_atom_identifier(first, dest)?;
                     for item in iter {
                         dest.write_str(", ")?;
-                        serialize_identifier(&item.to_string(), dest)?;
+                        serialize_atom_identifier(item, dest)?;
                     }
                 }
                 dest.write_char(')')?;

components/style/gecko/pseudo_element.rs

@@ -8,13 +8,14 @@
 //! `pseudo_element_definition.mako.rs`. If you touch that file, you probably
 //! need to update the checked-in files for Servo.
 
-use cssparser::{ToCss, serialize_identifier};
+use cssparser::ToCss;
 use gecko_bindings::structs::{self, CSSPseudoElementType};
 use properties::{CascadeFlags, ComputedValues, PropertyFlags};
 use properties::longhands::display::computed_value::T as Display;
 use selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl};
 use std::fmt;
 use string_cache::Atom;
+use values::serialize_atom_identifier;
 
 include!(concat!(env!("OUT_DIR"), "/gecko/pseudo_element_definition.rs"));
 

components/style/gecko/pseudo_element_definition.mako.rs

@@ -277,10 +277,10 @@ impl ToCss for PseudoElement {
                 dest.write_char('(')?;
                 let mut iter = args.iter();
                 if let Some(first) = iter.next() {
-                    serialize_identifier(&first.to_string(), dest)?;
+                    serialize_atom_identifier(&first, dest)?;
                     for item in iter {
                         dest.write_str(", ")?;
-                        serialize_identifier(&item.to_string(), dest)?;
+                        serialize_atom_identifier(item, dest)?;
                     }
                 }
                 dest.write_char(')')?;

components/style/gecko_string_cache/mod.rs

@@ -13,15 +13,14 @@ use gecko_bindings::bindings::Gecko_ReleaseAtom;
 use gecko_bindings::structs::{nsAtom, nsAtom_AtomKind, nsStaticAtom};
 use nsstring::{nsAString, nsStr};
 use precomputed_hash::PrecomputedHash;
+use std::{mem, slice, str};
 #[allow(unused_imports)] use std::ascii::AsciiExt;
 use std::borrow::{Cow, Borrow};
 use std::char::{self, DecodeUtf16};
 use std::fmt::{self, Write};
 use std::hash::{Hash, Hasher};
 use std::iter::Cloned;
-use std::mem;
 use std::ops::Deref;
-use std::slice;
 
 #[macro_use]
 #[allow(improper_ctypes, non_camel_case_types, missing_docs)]
@@ -128,21 +127,38 @@ impl WeakAtom {
     /// Find alternatives to this function when possible, please, since it's
     /// pretty slow.
     pub fn with_str<F, Output>(&self, cb: F) -> Output
-        where F: FnOnce(&str) -> Output
+    where
+        F: FnOnce(&str) -> Output
     {
-        // FIXME(bholley): We should measure whether it makes more sense to
-        // cache the UTF-8 version in the Gecko atom table somehow.
-        let owned = self.to_string();
-        cb(&owned)
-    }
+        let mut buffer: [u8; 64] = unsafe { mem::uninitialized() };
 
-    /// Convert this Atom into a string, decoding the UTF-16 bytes.
-    ///
-    /// Find alternatives to this function when possible, please, since it's
-    /// pretty slow.
-    #[inline]
-    pub fn to_string(&self) -> String {
-        String::from_utf16(self.as_slice()).unwrap()
+        // The total string length in utf16 is going to be less than or equal
+        // the slice length (each utf16 character is going to take at least one
+        // and at most 2 items in the utf16 slice).
+        //
+        // Each of those characters will take at most four bytes in the utf8
+        // one. Thus if the slice is less than 64 / 4 (16) we can guarantee that
+        // we'll decode it in place.
+        let owned_string;
+        let len = self.len();
+        let utf8_slice = if len <= 16 {
+            let mut total_len = 0;
+
+            for c in self.chars() {
+                let c = c.unwrap_or(char::REPLACEMENT_CHARACTER);
+                let utf8_len = c.encode_utf8(&mut buffer[total_len..]).len();
+                total_len += utf8_len;
+            }
+
+            let slice = unsafe { str::from_utf8_unchecked(&buffer[..total_len]) };
+            debug_assert_eq!(slice, String::from_utf16_lossy(self.as_slice()));
+            slice
+        } else {
+            owned_string = String::from_utf16_lossy(self.as_slice());
+            &*owned_string
+        };
+
+        cb(utf8_slice)
     }
 
     /// Returns whether this atom is static.

components/style/lib.rs

@@ -68,6 +68,7 @@ pub extern crate servo_arc;
 #[cfg(feature = "servo")] extern crate servo_url;
 extern crate smallbitvec;
 extern crate smallvec;
+#[cfg(feature = "servo")] extern crate string_cache;
 #[macro_use]
 extern crate style_derive;
 extern crate style_traits;

components/style/stylesheets/font_feature_values_rule.rs

@@ -8,7 +8,7 @@
 
 use Atom;
 use cssparser::{AtRuleParser, AtRuleType, BasicParseErrorKind, DeclarationListParser, DeclarationParser, Parser};
-use cssparser::{CowRcStr, RuleListParser, SourceLocation, QualifiedRuleParser, Token, serialize_identifier};
+use cssparser::{CowRcStr, RuleListParser, SourceLocation, QualifiedRuleParser, Token};
 use error_reporting::{ContextualParseError, ParseErrorReporter};
 #[cfg(feature = "gecko")]
 use gecko_bindings::bindings::Gecko_AppendFeatureValueHashEntry;
@@ -21,6 +21,7 @@ use str::CssStringWriter;
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 use stylesheets::CssRuleType;
 use values::computed::font::FamilyName;
+use values::serialize_atom_identifier;
 
 /// A @font-feature-values block declaration.
 /// It is `<ident>: <integer>+`.
@@ -41,7 +42,7 @@ impl<T: ToCss> ToCss for FFVDeclaration<T> {
     where
         W: Write,
     {
-        serialize_identifier(&self.name.to_string(), dest)?;
+        serialize_atom_identifier(&self.name, dest)?;
         dest.write_str(": ")?;
         self.value.to_css(dest)?;
         dest.write_str(";")

components/style/values/computed/font.rs

@@ -461,6 +461,7 @@ impl SingleFontFamily {
     /// Get the corresponding font-family with family name
     fn from_font_family_name(family: &structs::FontFamilyName) -> SingleFontFamily {
         use gecko_bindings::structs::FontFamilyType;
+        use values::serialize_atom_identifier;
 
         match family.mType {
             FontFamilyType::eFamily_sans_serif => SingleFontFamily::Generic(atom!("sans-serif")),
@@ -472,7 +473,7 @@ impl SingleFontFamily {
             FontFamilyType::eFamily_named => {
                 let name = Atom::from(&*family.mName);
                 let mut serialization = String::new();
-                serialize_identifier(&name.to_string(), &mut serialization).unwrap();
+                serialize_atom_identifier(&name, &mut serialization).unwrap();
                 SingleFontFamily::FamilyName(FamilyName {
                     name: name.clone(),
                     syntax: FamilyNameSyntax::Identifiers(serialization),

components/style/values/computed/mod.rs

@@ -4,7 +4,9 @@
 
 //! Computed values.
 
-use {Atom, Namespace};
+use Atom;
+#[cfg(feature = "servo")]
+use Prefix;
 use context::QuirksMode;
 use euclid::Size2D;
 use font_metrics::{FontMetricsProvider, get_metrics_provider_for_product};
@@ -414,7 +416,8 @@ trivial_to_computed_value!(u32);
 trivial_to_computed_value!(Atom);
 trivial_to_computed_value!(BorderStyle);
 trivial_to_computed_value!(CursorKind);
-trivial_to_computed_value!(Namespace);
+#[cfg(feature = "servo")]
+trivial_to_computed_value!(Prefix);
 trivial_to_computed_value!(String);
 trivial_to_computed_value!(Box<str>);
 

components/style/values/generics/image.rs

@@ -7,11 +7,11 @@
 //! [images]: https://drafts.csswg.org/css-images/#image-values
 
 use Atom;
-use cssparser::serialize_identifier;
 use custom_properties;
 use servo_arc::Arc;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
+use values::serialize_atom_identifier;
 
 /// An [image].
 ///
@@ -151,7 +151,7 @@ impl ToCss for PaintWorklet {
         W: Write,
     {
         dest.write_str("paint(")?;
-        serialize_identifier(&*self.name.to_string(), dest)?;
+        serialize_atom_identifier(&self.name, dest)?;
         for argument in &self.arguments {
             dest.write_str(", ")?;
             argument.to_css(dest)?;
@@ -200,7 +200,7 @@ impl<G, R, U> ToCss for Image<G, R, U>
             Image::PaintWorklet(ref paint_worklet) => paint_worklet.to_css(dest),
             Image::Element(ref selector) => {
                 dest.write_str("-moz-element(#")?;
-                serialize_identifier(&selector.to_string(), dest)?;
+                serialize_atom_identifier(selector, dest)?;
                 dest.write_str(")")
             },
         }

components/style/values/mod.rs

@@ -33,6 +33,25 @@ define_keyword_type!(None_, "none");
 define_keyword_type!(Auto, "auto");
 define_keyword_type!(Normal, "normal");
 
+/// Serialize an identifier which is represented as an atom.
+#[cfg(feature = "gecko")]
+pub fn serialize_atom_identifier<W>(ident: &Atom, dest: &mut W) -> fmt::Result
+where
+    W: Write,
+{
+    ident.with_str(|s| serialize_identifier(s, dest))
+}
+
+/// Serialize an identifier which is represented as an atom.
+#[cfg(feature = "servo")]
+pub fn serialize_atom_identifier<Static, W>(ident: &::string_cache::Atom<Static>, dest: &mut W) -> fmt::Result
+where
+    Static: ::string_cache::StaticAtomSet,
+    W: Write,
+{
+    serialize_identifier(&ident, dest)
+}
+
 /// Serialize a normalized value into percentage.
 pub fn serialize_percentage<W>(value: CSSFloat, dest: &mut CssWriter<W>) -> fmt::Result
 where
@@ -114,7 +133,7 @@ impl ToCss for CustomIdent {
     where
         W: Write,
     {
-        serialize_identifier(&self.0.to_string(), dest)
+        serialize_atom_identifier(&self.0, dest)
     }
 }
 

components/style/values/specified/mod.rs

@@ -6,7 +6,7 @@
 //!
 //! TODO(emilio): Enhance docs.
 
-use Namespace;
+use Prefix;
 use context::QuirksMode;
 use cssparser::{Parser, Token, serialize_identifier};
 use num_traits::One;
@@ -22,6 +22,7 @@ use super::computed::{Context, ToComputedValue};
 use super::generics::{GreaterThanOrEqualToOne, NonNegative};
 use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth};
 use super::generics::grid::{TrackSize as GenericTrackSize, TrackList as GenericTrackList};
+use values::serialize_atom_identifier;
 use values::specified::calc::CalcNode;
 
 pub use properties::animated_properties::TransitionProperty;
@@ -748,8 +749,8 @@ pub type NamespaceId = ();
 /// `[namespace? `|`]? ident`
 #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
 pub struct Attr {
-    /// Optional namespace
-    pub namespace: Option<(Namespace, NamespaceId)>,
+    /// Optional namespace prefix, with the actual namespace id.
+    pub namespace: Option<(Prefix, NamespaceId)>,
     /// Attribute name
     pub attribute: String,
 }
@@ -761,35 +762,18 @@ impl Parse for Attr {
     }
 }
 
-#[cfg(feature = "gecko")]
-/// Get the namespace id from the namespace map
-fn get_id_for_namespace(namespace: &Namespace, context: &ParserContext) -> Result<NamespaceId, ()> {
-    let namespaces_map = match context.namespaces {
-        Some(map) => map,
-        None => {
-            // If we don't have a namespace map (e.g. in inline styles)
-            // we can't parse namespaces
-            return Err(());
-        }
-    };
-
-    match namespaces_map.prefixes.get(&namespace.0) {
-        Some(entry) => Ok(entry.1),
-        None => Err(()),
-    }
-}
-
-#[cfg(feature = "servo")]
-/// Get the namespace id from the namespace map
-fn get_id_for_namespace(_: &Namespace, _: &ParserContext) -> Result<NamespaceId, ()> {
-    Ok(())
+/// Get the Namespace id from the namespace map.
+fn get_id_for_namespace(prefix: &Prefix, context: &ParserContext) -> Option<NamespaceId> {
+    Some(context.namespaces.as_ref()?.prefixes.get(prefix)?.1)
 }
 
 impl Attr {
     /// Parse contents of attr() assuming we have already parsed `attr` and are
     /// within a parse_nested_block()
-    pub fn parse_function<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
-                                  -> Result<Attr, ParseError<'i>> {
+    pub fn parse_function<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Attr, ParseError<'i>> {
         // Syntax is `[namespace? `|`]? ident`
         // no spaces allowed
         let first = input.try(|i| i.expect_ident_cloned()).ok();
@@ -804,11 +788,14 @@ impl Attr {
                     };
 
                     let ns_with_id = if let Some(ns) = first {
-                        let ns = Namespace::from(ns.as_ref());
-                        let id: Result<_, ParseError> =
-                            get_id_for_namespace(&ns, context)
-                            .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                        Some((ns, id?))
+                        let ns = Prefix::from(ns.as_ref());
+                        let id = match get_id_for_namespace(&ns, context) {
+                            Some(id) => id,
+                            None => return Err(location.new_custom_error(
+                                StyleParseErrorKind::UnspecifiedError
+                            )),
+                        };
+                        Some((ns, id))
                     } else {
                         None
                     };
@@ -819,7 +806,7 @@ impl Attr {
                 }
                 // In the case of attr(foobar    ) we don't want to error out
                 // because of the trailing whitespace
-                Token::WhiteSpace(_) => (),
+                Token::WhiteSpace(..) => {},
                 ref t => return Err(input.new_unexpected_token_error(t.clone())),
             }
         }
@@ -841,8 +828,8 @@ impl ToCss for Attr {
         W: Write,
     {
         dest.write_str("attr(")?;
-        if let Some(ref ns) = self.namespace {
-            serialize_identifier(&ns.0.to_string(), dest)?;
+        if let Some((ref prefix, _id)) = self.namespace {
+            serialize_atom_identifier(prefix, dest)?;
             dest.write_str("|")?;
         }
         serialize_identifier(&self.attribute, dest)?;