From f58301ecbc2f61cc6a42a2539f1123ab8bc1db8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 17 Dec 2020 14:04:35 +0000 Subject: [PATCH] style: Avoid UTF-8 -> UTF-16 conversion during CSSOM serialization. This lifts a bunch of string conversions higher up the stack, but allows us to make the servo code use utf-8 unconditionally, and seemed faster in my benchmarking (see comment 0). It should also make a bunch of attribute setters faster too (like setting .cssText), now that we use UTF8String for them (we couldn't because we couldn't specify different string types for the getter and setters). Differential Revision: https://phabricator.services.mozilla.com/D99590 --- .../style/properties/declaration_block.rs | 12 ++- .../style/properties/properties.mako.rs | 9 +- components/style/str.rs | 95 +------------------ 3 files changed, 15 insertions(+), 101 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index bd0a87f0b0a..9170f6e650d 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -14,7 +14,7 @@ use crate::parser::ParserContext; use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; use crate::selector_parser::SelectorImpl; use crate::shared_lock::Locked; -use crate::str::{CssString, CssStringBorrow, CssStringWriter}; +use crate::str::{CssString, CssStringWriter}; use crate::stylesheets::{CssRuleType, Origin, UrlExtraData}; use crate::values::computed::Context; use cssparser::{parse_important, CowRcStr, DeclarationListParser, ParserInput}; @@ -1093,7 +1093,11 @@ impl PropertyDeclarationBlock { } AppendableValue::Css { - css: CssStringBorrow::from(&v), + // Safety: serialization only generates valid utf-8. + #[cfg(feature = "gecko")] + css: unsafe { v.as_str_unchecked() }, + #[cfg(feature = "servo")] + css: &v, with_variables: false, } }, @@ -1179,7 +1183,7 @@ where /// or when storing a serialized shorthand value before appending directly. Css { /// The raw CSS string. - css: CssStringBorrow<'a>, + css: &'a str, /// Whether the original serialization contained variables or not. with_variables: bool, }, @@ -1207,7 +1211,7 @@ where I: Iterator, { match appendable_value { - AppendableValue::Css { css, .. } => css.append_to(dest), + AppendableValue::Css { css, .. } => dest.write_str(css), AppendableValue::Declaration(decl) => decl.to_css(dest), AppendableValue::DeclarationsForShorthand(shorthand, decls) => { shorthand.longhands_to_css(decls, &mut CssWriter::new(dest)) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index a4316a501ad..505068b8556 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -46,7 +46,7 @@ use crate::values::computed::NonNegativeLength; use crate::values::serialize_atom_name; use crate::rule_tree::StrongRuleNode; use crate::Zero; -use crate::str::{CssString, CssStringBorrow, CssStringWriter}; +use crate::str::{CssString, CssStringWriter}; use std::cell::Cell; pub use self::declaration_block::*; @@ -1529,10 +1529,7 @@ impl ShorthandId { // https://drafts.csswg.org/css-variables/#variables-in-shorthands if let Some(css) = first_declaration.with_variables_from_shorthand(self) { if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) { - return Some(AppendableValue::Css { - css: CssStringBorrow::from(css), - with_variables: true, - }); + return Some(AppendableValue::Css { css, with_variables: true }); } return None; } @@ -1541,7 +1538,7 @@ impl ShorthandId { if let Some(keyword) = first_declaration.get_css_wide_keyword() { if declarations2.all(|d| d.get_css_wide_keyword() == Some(keyword)) { return Some(AppendableValue::Css { - css: CssStringBorrow::from(keyword.to_str()), + css: keyword.to_str(), with_variables: false, }); } diff --git a/components/style/str.rs b/components/style/str.rs index 01f70c1f963..9badcdf413c 100644 --- a/components/style/str.rs +++ b/components/style/str.rs @@ -9,7 +9,6 @@ use num_traits::ToPrimitive; use std::borrow::Cow; use std::convert::AsRef; -use std::fmt::{self, Write}; use std::iter::{Filter, Peekable}; use std::str::Split; @@ -171,98 +170,12 @@ pub fn string_as_ascii_lowercase<'a>(input: &'a str) -> Cow<'a, str> { /// To avoid accidentally instantiating multiple monomorphizations of large /// serialization routines, we define explicit concrete types and require -/// them in those routines. This primarily avoids accidental mixing of UTF8 -/// with UTF16 serializations in Gecko. +/// them in those routines. This avoids accidental mixing of String and +/// nsACString arguments in Gecko, which would cause code size to blow up. #[cfg(feature = "gecko")] -pub type CssStringWriter = ::nsstring::nsAString; +pub type CssStringWriter = ::nsstring::nsACString; /// String type that coerces to CssStringWriter, used when serialization code /// needs to allocate a temporary string. #[cfg(feature = "gecko")] -pub type CssString = ::nsstring::nsString; - -/// Certain serialization code needs to interact with borrowed strings, which -/// are sometimes native UTF8 Rust strings, and other times serialized UTF16 -/// strings. This enum multiplexes the two cases. -#[cfg(feature = "gecko")] -pub enum CssStringBorrow<'a> { - /// A borrow of a UTF16 CssString. - UTF16(&'a ::nsstring::nsString), - /// A borrow of a regular Rust UTF8 string. - UTF8(&'a str), -} - -#[cfg(feature = "gecko")] -impl<'a> CssStringBorrow<'a> { - /// Writes the borrowed string to the provided writer. - pub fn append_to(&self, dest: &mut CssStringWriter) -> fmt::Result { - match *self { - CssStringBorrow::UTF16(s) => { - dest.append(s); - Ok(()) - }, - CssStringBorrow::UTF8(s) => dest.write_str(s), - } - } - - /// Returns true of the borrowed string is empty. - pub fn is_empty(&self) -> bool { - match *self { - CssStringBorrow::UTF16(s) => s.is_empty(), - CssStringBorrow::UTF8(s) => s.is_empty(), - } - } -} - -#[cfg(feature = "gecko")] -impl<'a> From<&'a str> for CssStringBorrow<'a> { - fn from(s: &'a str) -> Self { - CssStringBorrow::UTF8(s) - } -} - -#[cfg(feature = "gecko")] -impl<'a> From<&'a ::nsstring::nsString> for CssStringBorrow<'a> { - fn from(s: &'a ::nsstring::nsString) -> Self { - CssStringBorrow::UTF16(s) - } -} - -/// String. The comments for the Gecko types explain the need for this abstraction. -#[cfg(feature = "servo")] -pub type CssStringWriter = String; - -/// String. The comments for the Gecko types explain the need for this abstraction. -#[cfg(feature = "servo")] -pub type CssString = String; - -/// Borrowed string. The comments for the Gecko types explain the need for this abstraction. -#[cfg(feature = "servo")] -pub struct CssStringBorrow<'a>(&'a str); - -#[cfg(feature = "servo")] -impl<'a> CssStringBorrow<'a> { - /// Appends the borrowed string to the given string. - pub fn append_to(&self, dest: &mut CssStringWriter) -> fmt::Result { - dest.write_str(self.0) - } - - /// Returns true if the borrowed string is empty. - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } -} - -#[cfg(feature = "servo")] -impl<'a> From<&'a str> for CssStringBorrow<'a> { - fn from(s: &'a str) -> Self { - CssStringBorrow(s) - } -} - -#[cfg(feature = "servo")] -impl<'a> From<&'a String> for CssStringBorrow<'a> { - fn from(s: &'a String) -> Self { - CssStringBorrow(&*s) - } -} +pub type CssString = ::nsstring::nsCString;