From 8b20d7a98232fd8007fce4296fbd48503e57efa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 14 Jun 2017 11:40:49 +0200 Subject: [PATCH] style: Avoid quadratic time serialization of a declaration block. At least when the longhands aren't custom properties. We should also look into not serializing the style attribute eagerly when it's not needed... But a lot of code currently rely on attribute values being dereferenciables to &str, so that's harder to fix. We should really look into all those vectors around too, but that's probably less urgent. --- .../style/properties/declaration_block.rs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 0cc0aeca2fc..c332b4b07da 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -590,7 +590,7 @@ impl ToCss for PropertyDeclarationBlock { // Step 1 -> dest = result list // Step 2 - let mut already_serialized = Vec::new(); + let mut already_serialized = PropertyDeclarationIdSet::new(); // Step 3 for &(ref declaration, importance) in &*self.declarations { @@ -598,7 +598,7 @@ impl ToCss for PropertyDeclarationBlock { let property = declaration.id(); // Step 3.2 - if already_serialized.contains(&property) { + if already_serialized.contains(property) { continue; } @@ -606,8 +606,10 @@ impl ToCss for PropertyDeclarationBlock { let shorthands = declaration.shorthands(); if !shorthands.is_empty() { // Step 3.3.1 + // + // FIXME(emilio): All this looks terribly inefficient... let mut longhands = self.declarations.iter() - .filter(|d| !already_serialized.contains(&d.0.id())) + .filter(|d| !already_serialized.contains(d.0.id())) .collect::>(); // Step 3.3.2 @@ -642,10 +644,10 @@ impl ToCss for PropertyDeclarationBlock { } // Substep 1: // - // Assuming that the PropertyDeclarationBlock contains no - // duplicate entries, if the current_longhands length is - // equal to the properties length, it means that the - // properties that map to shorthand are present in longhands + // Assuming that the PropertyDeclarationBlock contains no + // duplicate entries, if the current_longhands length is + // equal to the properties length, it means that the + // properties that map to shorthand are present in longhands if current_longhands.len() != properties.len() { continue; } @@ -725,7 +727,7 @@ impl ToCss for PropertyDeclarationBlock { for current_longhand in ¤t_longhands { // Substep 9 - already_serialized.push(current_longhand.id()); + already_serialized.insert(current_longhand.id()); let index_to_remove = longhands.iter().position(|l| l.0 == **current_longhand); if let Some(index) = index_to_remove { // Substep 10 @@ -736,7 +738,7 @@ impl ToCss for PropertyDeclarationBlock { } // Step 3.3.4 - if already_serialized.contains(&property) { + if already_serialized.contains(property) { continue; } @@ -756,7 +758,7 @@ impl ToCss for PropertyDeclarationBlock { &mut is_first_serialization)?; // Step 3.3.8 - already_serialized.push(property); + already_serialized.insert(property); } // Step 4