Bug 1473180 part 2 - Add new algorithm for setting property to be used in later commit. r=emilio
authorXidorn Quan <me@upsuper.org>
Thu, 19 Jul 2018 10:11:04 +1000
changeset 427197 b863c916b7e5cd98524eb32d64be4829a476c951
parent 427196 dd7e1a3601963937161bad3b1bb730c8fa918dc8
child 427198 bd916a453f878fd15de301eaabb652779755fc11
push id105420
push userxquan@mozilla.com
push dateThu, 19 Jul 2018 00:14:53 +0000
treeherdermozilla-inbound@d487d2a5bc3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1473180
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1473180 part 2 - Add new algorithm for setting property to be used in later commit. r=emilio MozReview-Commit-ID: CdM8hDB6rFj
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/properties.mako.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -6,16 +6,17 @@
 
 #![deny(missing_docs)]
 
 use context::QuirksMode;
 use cssparser::{DeclarationListParser, parse_important, ParserInput, CowRcStr};
 use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter, ParseErrorKind};
 use custom_properties::CustomPropertiesBuilder;
 use error_reporting::{ParseErrorReporter, ContextualParseError};
+use itertools::Itertools;
 use parser::ParserContext;
 use properties::animated_properties::{AnimationValue, AnimationValueMap};
 use shared_lock::Locked;
 use smallbitvec::{self, SmallBitVec};
 use smallvec::SmallVec;
 use std::fmt::{self, Write};
 use std::iter::{DoubleEndedIterator, Zip};
 use std::slice::Iter;
@@ -34,16 +35,40 @@ pub struct AnimationRules(pub Option<Arc
 
 impl AnimationRules {
     /// Returns whether these animation rules represents an actual rule or not.
     pub fn is_empty(&self) -> bool {
         self.0.is_none() && self.1.is_none()
     }
 }
 
+/// An enum describes how a declaration should update
+/// the declaration block.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum DeclarationUpdate {
+    /// The given declaration doesn't update anything.
+    None,
+    /// The given declaration is new, and should be append directly.
+    Append,
+    /// The given declaration can be updated in-place at the given position.
+    UpdateInPlace { pos: usize },
+    /// The given declaration cannot be updated in-place, and an existing
+    /// one needs to be removed at the given position.
+    AppendAndRemove { pos: usize },
+}
+
+/// A struct describes how a declaration block should be updated by
+/// a `SourcePropertyDeclaration`.
+#[derive(Default)]
+pub struct SourcePropertyDeclarationUpdate {
+    updates: SubpropertiesVec<DeclarationUpdate>,
+    new_count: usize,
+    any_removal: bool,
+}
+
 /// Enum for how a given declaration should be pushed into a declaration block.
 #[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq)]
 pub enum DeclarationPushMode {
     /// Mode used when declarations were obtained from CSS parsing.
     /// If there is an existing declaration of the same property with a higher
     /// importance, the new declaration will be discarded. Otherwise, it will
     /// be appended to the end of the declaration block.
     Parsing,
@@ -435,20 +460,17 @@ impl PropertyDeclarationBlock {
         }
     }
 
     /// Returns whether the property is definitely new for this declaration
     /// block. It returns true when the declaration is a non-custom longhand
     /// and it doesn't exist in the block, and returns false otherwise.
     #[inline]
     fn is_definitely_new(&self, decl: &PropertyDeclaration) -> bool {
-        match decl.id() {
-            PropertyDeclarationId::Longhand(id) => !self.longhands.contains(id),
-            PropertyDeclarationId::Custom(..) => false,
-        }
+        decl.id().as_longhand().map_or(false, |id| !self.longhands.contains(id))
     }
 
     /// Returns whether calling extend with `DeclarationPushMode::Update`
     /// will cause this declaration block to change.
     pub fn will_change_in_update_mode(
         &self,
         source_declarations: &SourcePropertyDeclaration,
         importance: Importance,
@@ -591,16 +613,193 @@ impl PropertyDeclarationBlock {
         if let PropertyDeclarationId::Longhand(id) = declaration.id() {
             self.longhands.insert(id);
         }
         self.declarations.push(declaration);
         self.declarations_importance.push(importance.important());
         true
     }
 
+    /// Prepares updating this declaration block with the given
+    /// `SourcePropertyDeclaration` and importance, and returns whether
+    /// there is something to update.
+    pub fn prepare_for_update(
+        &self,
+        source_declarations: &SourcePropertyDeclaration,
+        importance: Importance,
+        updates: &mut SourcePropertyDeclarationUpdate,
+    ) -> bool {
+        debug_assert!(updates.updates.is_empty());
+        // Check whether we are updating for an all shorthand change.
+        if !matches!(source_declarations.all_shorthand, AllShorthand::NotSet) {
+            debug_assert!(source_declarations.declarations.is_empty());
+            return source_declarations.all_shorthand.declarations().any(|decl| {
+                self.is_definitely_new(&decl) ||
+                self.declarations.iter().enumerate()
+                    .find(|&(_, ref d)| d.id() == decl.id())
+                    .map_or(true, |(i, d)| {
+                        let important = self.declarations_importance[i];
+                        *d != decl || important != importance.important()
+                    })
+            });
+        }
+        // Fill `updates` with update information.
+        let mut any_update = false;
+        let new_count = &mut updates.new_count;
+        let any_removal = &mut updates.any_removal;
+        let updates = &mut updates.updates;
+        updates.extend(source_declarations.declarations.iter().map(|declaration| {
+            if self.is_definitely_new(declaration) {
+                return DeclarationUpdate::Append;
+            }
+            let longhand_id = declaration.id().as_longhand();
+            if let Some(longhand_id) = longhand_id {
+                if let Some(logical_group) = longhand_id.logical_group() {
+                    let mut needs_append = false;
+                    for (pos, decl) in self.declarations.iter().enumerate().rev() {
+                        let id = match decl.id().as_longhand() {
+                            Some(id) => id,
+                            None => continue,
+                        };
+                        if id == longhand_id {
+                            if needs_append {
+                                return DeclarationUpdate::AppendAndRemove { pos };
+                            }
+                            let important = self.declarations_importance[pos];
+                            if decl == declaration && important == importance.important() {
+                                return DeclarationUpdate::None;
+                            }
+                            return DeclarationUpdate::UpdateInPlace { pos };
+                        }
+                        if !needs_append &&
+                           id.logical_group() == Some(logical_group) &&
+                           id.is_logical() != longhand_id.is_logical() {
+                            needs_append = true;
+                        }
+                    }
+                    unreachable!("Longhand should be found in loop above");
+                }
+            }
+            self.declarations.iter().enumerate()
+                .find(|&(_, ref decl)| decl.id() == declaration.id())
+                .map_or(DeclarationUpdate::Append, |(pos, decl)| {
+                    let important = self.declarations_importance[pos];
+                    if decl == declaration && important == importance.important() {
+                        DeclarationUpdate::None
+                    } else {
+                        DeclarationUpdate::UpdateInPlace { pos }
+                    }
+                })
+        }).inspect(|update| {
+            if matches!(update, DeclarationUpdate::None) {
+                return;
+            }
+            any_update = true;
+            match update {
+                DeclarationUpdate::Append => {
+                    *new_count += 1;
+                }
+                DeclarationUpdate::AppendAndRemove { .. } => {
+                    *any_removal = true;
+                }
+                _ => {}
+            }
+        }));
+        any_update
+    }
+
+    /// Update this declaration block with the given data.
+    pub fn update(
+        &mut self,
+        drain: SourcePropertyDeclarationDrain,
+        importance: Importance,
+        updates: &mut SourcePropertyDeclarationUpdate,
+    ) {
+        let important = importance.important();
+        if !matches!(drain.all_shorthand, AllShorthand::NotSet) {
+            debug_assert!(updates.updates.is_empty());
+            for decl in drain.all_shorthand.declarations() {
+                if self.is_definitely_new(&decl) {
+                    let longhand_id = decl.id().as_longhand().unwrap();
+                    self.declarations.push(decl);
+                    self.declarations_importance.push(important);
+                    self.longhands.insert(longhand_id);
+                } else {
+                    let (idx, slot) = self.declarations.iter_mut()
+                        .enumerate().find(|&(_, ref d)| d.id() == decl.id()).unwrap();
+                    *slot = decl;
+                    self.declarations_importance.set(idx, important);
+                }
+            }
+            return;
+        }
+
+        self.declarations.reserve(updates.new_count);
+        if updates.any_removal {
+            // Prepare for removal and fixing update positions.
+            struct UpdateOrRemoval<'a> {
+                item: &'a mut DeclarationUpdate,
+                pos: usize,
+                remove: bool,
+            }
+            let mut updates_and_removals: SubpropertiesVec<UpdateOrRemoval> =
+                updates.updates.iter_mut().filter_map(|item| {
+                    let (pos, remove) = match *item {
+                        DeclarationUpdate::UpdateInPlace { pos } => (pos, false),
+                        DeclarationUpdate::AppendAndRemove { pos } => (pos, true),
+                        _ => return None,
+                    };
+                    Some(UpdateOrRemoval { item, pos, remove })
+                }).collect();
+            // Execute removals. It's important to do it in reverse index order,
+            // so that removing doesn't invalidate following positions.
+            updates_and_removals.sort_unstable_by_key(|update| update.pos);
+            updates_and_removals.iter().rev()
+                .filter(|update| update.remove)
+                .for_each(|update| {
+                    self.declarations.remove(update.pos);
+                    self.declarations_importance.remove(update.pos);
+                });
+            // Fixup pos field for updates.
+            let mut removed_count = 0;
+            for update in updates_and_removals.iter_mut() {
+                if update.remove {
+                    removed_count += 1;
+                    continue;
+                }
+                debug_assert_eq!(
+                    *update.item,
+                    DeclarationUpdate::UpdateInPlace { pos: update.pos }
+                );
+                *update.item = DeclarationUpdate::UpdateInPlace {
+                    pos: update.pos - removed_count
+                };
+            }
+        }
+        // Execute updates and appends.
+        for (decl, update) in drain.declarations.zip_eq(updates.updates.iter()) {
+            match *update {
+                DeclarationUpdate::None => {},
+                DeclarationUpdate::Append |
+                DeclarationUpdate::AppendAndRemove { .. } => {
+                    if let Some(id) = decl.id().as_longhand() {
+                        self.longhands.insert(id);
+                    }
+                    self.declarations.push(decl);
+                    self.declarations_importance.push(important);
+                }
+                DeclarationUpdate::UpdateInPlace { pos } => {
+                    self.declarations[pos] = decl;
+                    self.declarations_importance.set(pos, important);
+                }
+            }
+        }
+        updates.updates.clear();
+    }
+
     /// Returns the first declaration that would be removed by removing
     /// `property`.
     #[inline]
     pub fn first_declaration_to_remove(
         &self,
         property: &PropertyId,
     ) -> Option<usize> {
         if let Some(id) = property.longhand_id() {
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -7,16 +7,17 @@
 // Please note that valid Rust syntax may be mangled by the Mako parser.
 // For example, Vec<&Foo> will be mangled as Vec&Foo>. To work around these issues, the code
 // can be escaped. In the above example, Vec<<&Foo> or Vec< &Foo> achieves the desired result of Vec<&Foo>.
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 #[cfg(feature = "servo")]
 use app_units::Au;
+use arrayvec::{ArrayVec, Drain as ArrayVecDrain};
 use dom::TElement;
 use custom_properties::CustomPropertiesBuilder;
 use servo_arc::{Arc, UniqueArc};
 use smallbitvec::SmallBitVec;
 use std::borrow::Cow;
 use std::{ops, ptr};
 use std::cell::RefCell;
 use std::fmt::{self, Write};
@@ -1600,16 +1601,25 @@ impl<'a> PropertyDeclarationId<'a> {
             PropertyDeclarationId::Longhand(id) => id.name().into(),
             PropertyDeclarationId::Custom(name) => {
                 let mut s = String::new();
                 write!(&mut s, "--{}", name).unwrap();
                 s.into()
             }
         }
     }
+
+    /// Returns longhand id if it is, None otherwise.
+    #[inline]
+    pub fn as_longhand(&self) -> Option<LonghandId> {
+        match *self {
+            PropertyDeclarationId::Longhand(id) => Some(id),
+            _ => None,
+        }
+    }
 }
 
 /// Servo's representation of a CSS property, that is, either a longhand, a
 /// shorthand, or a custom property.
 #[derive(Clone, Eq, PartialEq)]
 pub enum PropertyId {
     /// A longhand property.
     Longhand(LonghandId),
@@ -2261,29 +2271,28 @@ impl PropertyDeclaration {
                         }
                     })
                 }
             }
         }
     }
 }
 
-const MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL: usize =
-    ${max(len(s.sub_properties) for s in data.shorthands_except_all())};
-
-type SourcePropertyDeclarationArray =
-    [PropertyDeclaration; MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL];
+type SubpropertiesArray<T> =
+    [T; ${max(len(s.sub_properties) for s in data.shorthands_except_all())}];
+
+type SubpropertiesVec<T> = ArrayVec<SubpropertiesArray<T>>;
 
 /// A stack-allocated vector of `PropertyDeclaration`
 /// large enough to parse one CSS `key: value` declaration.
 /// (Shorthands expand to multiple `PropertyDeclaration`s.)
 pub struct SourcePropertyDeclaration {
-    declarations: ::arrayvec::ArrayVec<SourcePropertyDeclarationArray>,
-
-    /// Stored separately to keep MAX_SUB_PROPERTIES_PER_SHORTHAND_EXCEPT_ALL smaller.
+    declarations: SubpropertiesVec<PropertyDeclaration>,
+
+    /// Stored separately to keep SubpropertiesVec smaller.
     all_shorthand: AllShorthand,
 }
 
 impl SourcePropertyDeclaration {
     /// Create one. It’s big, try not to move it around.
     #[inline]
     pub fn new() -> Self {
         SourcePropertyDeclaration {
@@ -2313,17 +2322,17 @@ impl SourcePropertyDeclaration {
     fn push(&mut self, declaration: PropertyDeclaration) {
         let _result = self.declarations.try_push(declaration);
         debug_assert!(_result.is_ok());
     }
 }
 
 /// Return type of SourcePropertyDeclaration::drain
 pub struct SourcePropertyDeclarationDrain<'a> {
-    declarations: ::arrayvec::Drain<'a, SourcePropertyDeclarationArray>,
+    declarations: ArrayVecDrain<'a, SubpropertiesArray<PropertyDeclaration>>,
     all_shorthand: AllShorthand,
 }
 
 enum AllShorthand {
     NotSet,
     CSSWideKeyword(CSSWideKeyword),
     WithVariables(Arc<UnparsedValue>)
 }