From a109fbb7c8dbcc234461b65f79e4e3f5f2ad56dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 16 May 2019 23:04:31 +0000 Subject: [PATCH] style: Use ArcSlice for quotes. This saves the intermediate allocation. Differential Revision: https://phabricator.services.mozilla.com/D30546 --- components/style/gecko/arc_types.rs | 5 -- components/style/lib.rs | 1 + components/style/properties/gecko.mako.rs | 24 +-------- components/style/values/computed/list.rs | 14 +++-- components/style/values/specified/list.rs | 20 ++++--- components/style_traits/lib.rs | 1 + components/style_traits/owned_str.rs | 53 +++++++++++++++++++ .../style_traits/specified_value_info.rs | 1 + components/style_traits/values.rs | 10 ++++ 9 files changed, 85 insertions(+), 44 deletions(-) create mode 100644 components/style_traits/owned_str.rs diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 309f2395085..7aed10b6568 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -23,7 +23,6 @@ use crate::gecko_bindings::structs::RawServoMediaRule; use crate::gecko_bindings::structs::RawServoMozDocumentRule; use crate::gecko_bindings::structs::RawServoNamespaceRule; use crate::gecko_bindings::structs::RawServoPageRule; -use crate::gecko_bindings::structs::RawServoQuotes; use crate::gecko_bindings::structs::RawServoStyleRule; use crate::gecko_bindings::structs::RawServoStyleSheetContents; use crate::gecko_bindings::structs::RawServoSupportsRule; @@ -40,7 +39,6 @@ use crate::stylesheets::{NamespaceRule, PageRule}; use crate::stylesheets::{StyleRule, StylesheetContents, SupportsRule}; use servo_arc::{Arc, ArcBorrow}; use std::{mem, ptr}; -use values::computed::QuotePair; macro_rules! impl_arc_ffi { ($servo_type:ty => $gecko_type:ty[$addref:ident, $release:ident]) => { @@ -115,9 +113,6 @@ impl_arc_ffi!(Locked => RawServoCounterStyleRule impl_arc_ffi!(CssUrlData => RawServoCssUrlData [Servo_CssUrlData_AddRef, Servo_CssUrlData_Release]); -impl_arc_ffi!(Box<[QuotePair]> => RawServoQuotes - [Servo_Quotes_AddRef, Servo_Quotes_Release]); - // ComputedStyle is not an opaque type on any side of FFI. // This means that doing the HasArcFFI type trick is actually unsound, // since it gives us a way to construct an Arc from diff --git a/components/style/lib.rs b/components/style/lib.rs index 4d523d65c4f..587b926efc6 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -190,6 +190,7 @@ pub use servo_atoms::Atom; pub use style_traits::arc_slice::ArcSlice; pub use style_traits::owned_slice::OwnedSlice; +pub use style_traits::owned_str::OwnedStr; /// The CSS properties supported by the style system. /// Generated from the properties.mako.rs template by build.rs diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 79a258b2d9a..6f37f889e6b 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3302,7 +3302,7 @@ fn static_assert() { <%self:impl_trait style_struct_name="List" - skip_longhands="list-style-image list-style-type quotes -moz-image-region"> + skip_longhands="list-style-image list-style-type -moz-image-region"> pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) { match image { @@ -3378,28 +3378,6 @@ fn static_assert() { } } - pub fn set_quotes(&mut self, other: longhands::quotes::computed_value::T) { - self.gecko.mQuotes.set_arc(other.0.clone()); - } - - pub fn copy_quotes_from(&mut self, other: &Self) { - self.set_quotes(other.clone_quotes()); - } - - pub fn reset_quotes(&mut self, other: &Self) { - self.copy_quotes_from(other) - } - - pub fn clone_quotes(&self) -> longhands::quotes::computed_value::T { - use gecko_bindings::sugar::ownership::HasArcFFI; - use values::computed::QuotePair; - - let quote_pairs = unsafe { &*self.gecko.mQuotes.mRawPtr }; - longhands::quotes::computed_value::T( - Box::<[QuotePair]>::as_arc("e_pairs).clone_arc() - ) - } - #[allow(non_snake_case)] pub fn set__moz_image_region(&mut self, v: longhands::_moz_image_region::computed_value::T) { use crate::values::Either; diff --git a/components/style/values/computed/list.rs b/components/style/values/computed/list.rs index 2bde35e3b6b..3536aa656fe 100644 --- a/components/style/values/computed/list.rs +++ b/components/style/values/computed/list.rs @@ -9,21 +9,19 @@ pub use crate::values::specified::list::ListStyleType; pub use crate::values::specified::list::MozListReversed; pub use crate::values::specified::list::{QuotePair, Quotes}; -use servo_arc::Arc; - lazy_static! { - static ref INITIAL_QUOTES: Arc> = Arc::new( + static ref INITIAL_QUOTES: crate::ArcSlice = crate::ArcSlice::from_iter( vec![ QuotePair { - opening: "\u{201c}".to_owned().into_boxed_str(), - closing: "\u{201d}".to_owned().into_boxed_str(), + opening: "\u{201c}".to_owned().into(), + closing: "\u{201d}".to_owned().into(), }, QuotePair { - opening: "\u{2018}".to_owned().into_boxed_str(), - closing: "\u{2019}".to_owned().into_boxed_str(), + opening: "\u{2018}".to_owned().into(), + closing: "\u{2019}".to_owned().into(), }, ] - .into_boxed_slice() + .into_iter() ); } diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs index 8c1e9fa2ad4..566b84bbb16 100644 --- a/components/style/values/specified/list.rs +++ b/components/style/values/specified/list.rs @@ -10,7 +10,6 @@ use crate::values::generics::CounterStyleOrNone; #[cfg(feature = "gecko")] use crate::values::CustomIdent; use cssparser::{Parser, Token}; -use servo_arc::Arc; use style_traits::{ParseError, StyleParseErrorKind}; /// Specified and computed `list-style-type` property. @@ -96,18 +95,20 @@ impl Parse for ListStyleType { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct QuotePair { /// The opening quote. - pub opening: Box, + pub opening: crate::OwnedStr, /// The closing quote. - pub closing: Box, + pub closing: crate::OwnedStr, } /// Specified and computed `quotes` property. #[derive( Clone, Debug, + Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, @@ -116,10 +117,11 @@ pub struct QuotePair { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct Quotes( #[css(iterable, if_empty = "none")] #[ignore_malloc_size_of = "Arc"] - pub Arc>, + pub crate::ArcSlice, ); impl Parse for Quotes { @@ -131,24 +133,26 @@ impl Parse for Quotes { .try(|input| input.expect_ident_matching("none")) .is_ok() { - return Ok(Quotes(Arc::new(Box::new([])))); + return Ok(Self::default()); } let mut quotes = Vec::new(); loop { let location = input.current_source_location(); let opening = match input.next() { - Ok(&Token::QuotedString(ref value)) => value.as_ref().to_owned().into_boxed_str(), + Ok(&Token::QuotedString(ref value)) => { + value.as_ref().to_owned().into() + }, Ok(t) => return Err(location.new_unexpected_token_error(t.clone())), Err(_) => break, }; - let closing = input.expect_string()?.as_ref().to_owned().into_boxed_str(); + let closing = input.expect_string()?.as_ref().to_owned().into(); quotes.push(QuotePair { opening, closing }); } if !quotes.is_empty() { - Ok(Quotes(Arc::new(quotes.into_boxed_slice()))) + Ok(Quotes(crate::ArcSlice::from_iter(quotes.into_iter()))) } else { Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } diff --git a/components/style_traits/lib.rs b/components/style_traits/lib.rs index 0ba13082fc3..cc87ff36cff 100644 --- a/components/style_traits/lib.rs +++ b/components/style_traits/lib.rs @@ -93,6 +93,7 @@ pub mod values; #[macro_use] pub mod viewport; pub mod owned_slice; +pub mod owned_str; pub use crate::specified_value_info::{CssType, KeywordsCollectFn, SpecifiedValueInfo}; pub use crate::values::{ diff --git a/components/style_traits/owned_str.rs b/components/style_traits/owned_str.rs new file mode 100644 index 00000000000..e5fe1065712 --- /dev/null +++ b/components/style_traits/owned_str.rs @@ -0,0 +1,53 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#![allow(unsafe_code)] + +//! A replacement for `Box` that has a defined layout for FFI. + +use crate::owned_slice::OwnedSlice; +use std::fmt; +use std::ops::{Deref, DerefMut}; + +/// A struct that basically replaces a Box, but with a defined layout, +/// suitable for FFI. +#[repr(C)] +#[derive(Default, Clone, PartialEq, Eq, MallocSizeOf, ToShmem)] +pub struct OwnedStr(OwnedSlice); + +impl fmt::Debug for OwnedStr { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + self.deref().fmt(formatter) + } +} + +impl Deref for OwnedStr { + type Target = str; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + unsafe { std::str::from_utf8_unchecked(&*self.0) } + } +} + +impl DerefMut for OwnedStr { + #[inline(always)] + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { std::str::from_utf8_unchecked_mut(&mut *self.0) } + } +} + +impl From> for OwnedStr { + #[inline] + fn from(b: Box) -> Self { + Self::from(b.into_string()) + } +} + +impl From for OwnedStr { + #[inline] + fn from(s: String) -> Self { + OwnedStr(s.into_bytes().into()) + } +} diff --git a/components/style_traits/specified_value_info.rs b/components/style_traits/specified_value_info.rs index 95f8821cef5..5aaf64ac955 100644 --- a/components/style_traits/specified_value_info.rs +++ b/components/style_traits/specified_value_info.rs @@ -84,6 +84,7 @@ impl SpecifiedValueInfo for u16 {} impl SpecifiedValueInfo for u32 {} impl SpecifiedValueInfo for str {} impl SpecifiedValueInfo for String {} +impl SpecifiedValueInfo for crate::owned_str::OwnedStr {} #[cfg(feature = "servo")] impl SpecifiedValueInfo for ::servo_atoms::Atom {} diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index 5c3f3ab255d..0c3166858a2 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -75,6 +75,16 @@ where } } +impl ToCss for crate::owned_str::OwnedStr { + #[inline] + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + serialize_string(self, dest) + } +} + impl ToCss for str { #[inline] fn to_css(&self, dest: &mut CssWriter) -> fmt::Result