Skip the colour regexp on comma-free shorthands, return sanitized media types as-is: −7% objects, −4% CPU

CHANGELOG.md

@@ -3,6 +3,8 @@
 ### Unreleased
 * Speed up `RuleSet::Declarations#to_s` ~20x: its clean-up regexp matched the empty string at every character (`[\n\r\f\t]*`), making `gsub!` replace at every position; the unreachable `\s+$` branch is dropped, output unchanged
 * Allocate less in `RuleSet`: skip the downcased copy of already normalized property names, and build dimension shorthands without enumerators
+* Allocate less when expanding and creating dimension shorthands, and skip the colour regexp on values without a comma
+* Return already sanitized media types as-is from `sanitize_media_query`, and allocate less when splitting selectors and counting ids
 
 ### Version 3.3.0
 * Fix Arbitrary local-file read via attacker-controlled `@import`

lib/css_parser.rb

@@ -107,7 +107,7 @@ def self.merge(*rule_sets)
   #++
   def self.calculate_specificity(selector)
     a = 0
-    b = selector.scan('#').length
+    b = selector.count('#')
     c = selector.scan(NON_ID_ATTRIBUTES_AND_PSEUDO_CLASSES_RX_NC).length
     d = selector.scan(ELEMENTS_AND_PSEUDO_ELEMENTS_RX_NC).length
 
@@ -147,6 +147,8 @@ def self.convert_uris(css, base_uri)
   end
 
   def self.sanitize_media_query(raw)
+    return raw if raw.is_a?(Symbol) && !raw.match?(UNSANITIZED_MEDIA_QUERY_RX)
+
     mq = raw.to_s.gsub(/\s+/, ' ')
     mq.strip!
     mq = 'all' if mq.empty?

lib/css_parser/regexps.rb

@@ -35,6 +35,9 @@ def self.regex_possible_values(*values)
   #++
   IMPORTANT_IN_PROPERTY_RX = /\s*!important\b\s*/i.freeze
 
+  # What sanitize_media_query would change: blank, padded, or not single-space separated
+  UNSANITIZED_MEDIA_QUERY_RX = /\A\z|\A[\s\0]|[\s\0]\z|\s\s|[^\S ]/.freeze
+
   RE_INSIDE_OUTSIDE = regex_possible_values 'inside', 'outside'
   RE_SCROLL_FIXED = regex_possible_values 'scroll', 'fixed'
   RE_REPEAT = regex_possible_values 'repeat(\-x|\-y)*|no\-repeat'

lib/css_parser/rule_set.rb

@@ -403,14 +403,14 @@ def expand_dimensions_shorthand! # :nodoc:
       DIMENSIONS.each do |property, (top, right, bottom, left)|
         next unless (declaration = declarations[property])
 
-        value = declaration.value.dup
+        value = declaration.value
 
         # RGB and HSL values in borders are the only units that can have spaces (within params).
         # We cheat a bit here by stripping spaces after commas in RGB and HSL values so that we
         # can split easily on spaces.
         #
         # TODO: rgba, hsl, hsla
-        value.gsub!(RE_COLOUR) { |c| c.gsub(/(\s*,\s*)/, ',') }
+        value = value.gsub(RE_COLOUR) { |c| c.gsub(/(\s*,\s*)/, ',') } if value.include?(',')
 
         matches = split_value_preserving_function_whitespace(value)
 
@@ -428,7 +428,7 @@ def expand_dimensions_shorthand! # :nodoc:
           raise ArgumentError, "Cannot parse #{value}"
         end
 
-        replacement = [top, right, bottom, left].zip(values).to_h
+        replacement = {top => values[0], right => values[1], bottom => values[2], left => values[3]}
 
         declarations.replace_declaration!(property, replacement, preserve_importance: true)
       end
@@ -522,17 +522,13 @@ def create_shorthand!
 
     # Combine several properties into a shorthand one
     def create_shorthand_properties!(properties, shorthand_property) # :nodoc:
-      values = []
-      properties_to_delete = []
-      properties.each do |property|
-        next unless (declaration = declarations[property])
-        next if declaration.important
-
-        values << declaration.value
-        properties_to_delete << property
+      properties_to_delete = properties.select do |property|
+        (declaration = declarations[property]) && !declaration.important
       end
 
-      return if values.length <= 1
+      return if properties_to_delete.length <= 1
+
+      values = properties_to_delete.map { |property| declarations[property].value }
 
       properties_to_delete.each do |property|
         declarations.delete(property)
@@ -588,15 +584,11 @@ def create_dimensions_shorthand! # :nodoc:
       return if declarations.size < NUMBER_OF_DIMENSIONS
 
       DIMENSIONS.each do |property, dimensions|
-        values = {}
-        DIMENSION_DIRECTIONS.each_with_index do |side, index|
-          next unless (declaration = declarations[dimensions[index]])
-
-          values[side] = declaration.value
-        end
-
         # All four dimensions must be present
-        next if values.size != dimensions.size
+        next unless dimensions.all? { |dimension| declarations.key?(dimension) }
+
+        top, right, bottom, left = dimensions.map { |dimension| declarations[dimension].value }
+        values = {top: top, right: right, bottom: bottom, left: left}
 
         new_value = values.values_at(*compute_dimensions_shorthand(values)).join(' ').strip
         declarations[property] = new_value unless new_value.empty?
@@ -646,7 +638,7 @@ def create_list_style_shorthand! # :nodoc:
 
     def compute_dimensions_shorthand(values)
       # All four sides are equal, returning single value
-      return [:top] if values.values.uniq.count == 1
+      return [:top] if values[:top] == values[:right] && values[:right] == values[:bottom] && values[:bottom] == values[:left]
 
       # `/* top | right | bottom | left */`
       return DIMENSION_DIRECTIONS if values[:left] != values[:right]
@@ -693,7 +685,7 @@ def unmatched_open_parenthesis?(declarations)
     # TODO: way too simplistic
     #++
     def parse_selectors!(selectors) # :nodoc:
-      @selectors = selectors.split(',').map do |s|
+      @selectors = selectors.split(',').map! do |s|
         s.gsub!(/\s+/, ' ')
         s.strip!
         s

test/test_css_parser_media_types.rb

@@ -5,6 +5,7 @@
 # Test cases for the handling of media types
 class CssParserMediaTypesTests < Minitest::Test
   include CssParser
+  include AllocationCounting
 
   def setup
     @cp = Parser.new
@@ -155,4 +156,8 @@ def test_to_s_includes_media_queries
     @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: 'aural and (device-aspect-ratio: 16/9)')
     assert_equal "@media aural and (device-aspect-ratio: 16/9) {\n  body {\n    color: black;\n  }\n}\n", @cp.to_s
   end
+
+  def test_sanitizing_an_already_sanitized_media_type_does_not_allocate
+    assert_equal(0, allocations { CssParser.sanitize_media_query(:all) })
+  end
 end

test/test_helper.rb

@@ -7,3 +7,19 @@
 require 'net/http'
 require 'webrick'
 require 'css_parser'
+
+module AllocationCounting
+  def allocations
+    skip 'GC.stat(:total_allocated_objects) is MRI only' unless RUBY_ENGINE == 'ruby'
+
+    yield
+    GC.disable
+    Array.new(3) do
+      before = GC.stat(:total_allocated_objects)
+      yield
+      GC.stat(:total_allocated_objects) - before
+    end.min
+  ensure
+    GC.enable
+  end
+end

test/test_rule_set_creating_shorthand.rb

@@ -5,6 +5,7 @@
 # Test cases for reading and generating CSS shorthand properties
 class RuleSetCreatingShorthandTests < Minitest::Test
   include CssParser
+  include AllocationCounting
 
   def setup
     @cp = CssParser::Parser.new
@@ -220,6 +221,13 @@ def test_a_single_property_is_not_shorted
     assert_equal('', combined['background'])
   end
 
+  def test_creating_dimensions_shorthand_allocates_less_than_thirty_objects
+    block = 'margin-top: 1px; margin-right: 2px; margin-bottom: 1px; margin-left: 2px; padding-top: 0; padding-right: 0; padding-bottom: 0; padding-left: 0'
+    parsing = allocations { RuleSet.new(block: block) }
+
+    assert_operator allocations { RuleSet.new(block: block).create_shorthand! } - parsing, :<, 30
+  end
+
 protected
 
   def assert_properties_are_deleted(ruleset, properties)