From c87da27bca36af33f1ea2602d1e51dbf081f2674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 19 Jun 2019 05:58:11 +0000 Subject: [PATCH] style: Use atoms for grid line names. The style system already atomizes all CustomIdent values, which means that we're just wasting memory and CPU by doing string copies all over the place. This patch fixes it. This also simplifies further changes to use as much of the rust data structures as possible. I had to switch from nsTHashtable to mozilla::HashTable because the former doesn't handle well non-default-constructible structs (like NamedLine, which now has a StyleAtom, which is not default-constructible). Differential Revision: https://phabricator.services.mozilla.com/D35119 --- components/style/properties/gecko.mako.rs | 44 ++++++++++--------- components/style/values/specified/position.rs | 19 ++++---- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 1105e705060..fe0a1935e97 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1137,11 +1137,12 @@ fn static_assert() { use crate::gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine}; let line = &mut self.gecko.${value.gecko}; - let line_name = &mut line.mLineName; - match v.ident.as_ref() { - Some(i) => i.0.with_str(|s| line_name.assign(s)), - None => line_name.assign(""), - }; + line.mLineName.set_move(unsafe { + RefPtr::from_addrefed(match v.ident { + Some(i) => i.0, + None => atom!(""), + }.into_addrefed()) + }); line.mHasSpan = v.is_span; if let Some(integer) = v.line_num { // clamping the integer between a range @@ -1155,7 +1156,9 @@ fn static_assert() { pub fn copy_${value.name}_from(&mut self, other: &Self) { self.gecko.${value.gecko}.mHasSpan = other.gecko.${value.gecko}.mHasSpan; self.gecko.${value.gecko}.mInteger = other.gecko.${value.gecko}.mInteger; - self.gecko.${value.gecko}.mLineName.assign(&*other.gecko.${value.gecko}.mLineName); + unsafe { + self.gecko.${value.gecko}.mLineName.set(&other.gecko.${value.gecko}.mLineName); + } } pub fn reset_${value.name}(&mut self, other: &Self) { @@ -1168,11 +1171,11 @@ fn static_assert() { longhands::${value.name}::computed_value::T { is_span: self.gecko.${value.gecko}.mHasSpan, ident: { - let name = self.gecko.${value.gecko}.mLineName.to_string(); - if name.len() == 0 { + let name = unsafe { Atom::from_raw(self.gecko.${value.gecko}.mLineName.mRawPtr) }; + if name == atom!("") { None } else { - Some(CustomIdent(Atom::from(name))) + Some(CustomIdent(name)) } }, line_num: @@ -1211,20 +1214,21 @@ fn static_assert() { pub fn set_grid_template_${kind}(&mut self, v: longhands::grid_template_${kind}::computed_value::T) { <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %> use crate::gecko_bindings::structs::{nsTArray, nsStyleGridLine_kMaxLine}; - use nsstring::nsCString; use std::usize; use crate::values::CustomIdent; use crate::values::generics::grid::TrackListType::Auto; use crate::values::generics::grid::{GridTemplateComponent, RepeatCount}; #[inline] - fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray) { + fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray>) { unsafe { - bindings::Gecko_ResizeTArrayForCStrings(gecko_names, servo_names.len() as u32); + bindings::Gecko_ResizeAtomArray(gecko_names, servo_names.len() as u32); } for (servo_name, gecko_name) in servo_names.iter().zip(gecko_names.iter_mut()) { - servo_name.0.with_str(|s| gecko_name.assign(s)) + gecko_name.set_move(unsafe { + RefPtr::from_addrefed(servo_name.0.clone().into_addrefed()) + }); } } @@ -1262,9 +1266,9 @@ fn static_assert() { auto_track_size = Some(auto_repeat.track_sizes.get(0).unwrap().clone()); } else { unsafe { - bindings::Gecko_ResizeTArrayForCStrings( + bindings::Gecko_ResizeAtomArray( &mut value.mRepeatAutoLineNameListBefore, 0); - bindings::Gecko_ResizeTArrayForCStrings( + bindings::Gecko_ResizeAtomArray( &mut value.mRepeatAutoLineNameListAfter, 0); } } @@ -1338,7 +1342,6 @@ fn static_assert() { pub fn clone_grid_template_${kind}(&self) -> longhands::grid_template_${kind}::computed_value::T { <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %> use crate::gecko_bindings::structs::nsTArray; - use nsstring::nsCString; use crate::values::CustomIdent; use crate::values::generics::grid::{GridTemplateComponent, LineNameList, RepeatCount}; use crate::values::generics::grid::{TrackList, TrackListType, TrackListValue, TrackRepeat, TrackSize}; @@ -1349,16 +1352,17 @@ fn static_assert() { }; #[inline] - fn to_boxed_customident_slice(gecko_names: &nsTArray) -> Box<[CustomIdent]> { + fn to_boxed_customident_slice(gecko_names: &nsTArray>) -> Box<[CustomIdent]> { let idents: Vec = gecko_names.iter().map(|gecko_name| { - CustomIdent(Atom::from(unsafe { gecko_name.as_str_unchecked() })) + CustomIdent(unsafe { Atom::from_raw(gecko_name.mRawPtr) }) }).collect(); idents.into_boxed_slice() } #[inline] - fn to_line_names_vec(gecko_line_names: &nsTArray>) - -> Vec> { + fn to_line_names_vec( + gecko_line_names: &nsTArray>>, + ) -> Vec> { gecko_line_names.iter().map(|gecko_names| { to_boxed_customident_slice(gecko_names) }).collect() diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index dfce7218eed..aeffd418d27 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -7,7 +7,8 @@ //! //! [position]: https://drafts.csswg.org/css-backgrounds-3/#position -use crate::hash::FxHashMap; +use crate::Atom; +use crate::selector_map::PrecomputedHashMap; use crate::parser::{Parse, ParserContext}; use crate::str::HTML_SPACE_CHARACTERS; use crate::values::computed::LengthPercentage as ComputedLengthPercentage; @@ -500,15 +501,15 @@ impl TemplateAreas { let mut width = 0; { let mut row = 0u32; - let mut area_indices = FxHashMap::<&str, usize>::default(); + let mut area_indices = PrecomputedHashMap::::default(); for string in &strings { let mut current_area_index: Option = None; row += 1; let mut column = 0u32; for token in TemplateAreasTokenizer(string) { column += 1; - let token = if let Some(token) = token? { - token + let name = if let Some(token) = token? { + Atom::from(token) } else { if let Some(index) = current_area_index.take() { if areas[index].columns.end != column { @@ -518,7 +519,7 @@ impl TemplateAreas { continue; }; if let Some(index) = current_area_index { - if &*areas[index].name == token { + if areas[index].name == name { if areas[index].rows.start == row { areas[index].columns.end += 1; } @@ -528,7 +529,7 @@ impl TemplateAreas { return Err(()); } } - if let Some(index) = area_indices.get(token).cloned() { + if let Some(index) = area_indices.get(&name).cloned() { if areas[index].columns.start != column || areas[index].rows.end != row { return Err(()); } @@ -537,8 +538,9 @@ impl TemplateAreas { continue; } let index = areas.len(); + assert!(area_indices.insert(name.clone(), index).is_none()); areas.push(NamedArea { - name: token.to_owned().into(), + name, columns: UnsignedRange { start: column, end: column + 1, @@ -548,7 +550,6 @@ impl TemplateAreas { end: row + 1, }, }); - assert!(area_indices.insert(token, index).is_none()); current_area_index = Some(index); } if let Some(index) = current_area_index { @@ -629,7 +630,7 @@ pub struct UnsignedRange { /// grid-placement properties. pub struct NamedArea { /// Name of the `named area` - pub name: crate::OwnedStr, + pub name: Atom, /// Rows of the `named area` pub rows: UnsignedRange, /// Columns of the `named area`