From cfded39937be84687e4cab79f1d441b5c746b22b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Joubert?= Date: Mon, 26 Oct 2026 10:14:14 +0300 Subject: [PATCH 1/2] Allocate less in shorthands and media types, skip the colour regexp without commas expand_dimensions_shorthand! ran RE_COLOUR, the alternation of every named colour, over each dimension value only to collapse spaces around commas in rgb()/hsl(): without a comma that is the identity. Creating dimension and list/background shorthands stops building throwaway arrays and zip pairs. sanitize_media_query returns an already sanitized symbol as-is instead of round-tripping :all through to_s/gsub/strip!/to_sym for every rule; calculate_specificity counts ids with count('#') and RuleSet#initialize no longer builds a selectors array only to replace it. Inlining our 336 emails with premailer allocates 7% fewer objects and uses 4% less CPU, with byte-for-byte identical output. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ lib/css_parser.rb | 4 ++- lib/css_parser/regexps.rb | 3 ++ lib/css_parser/rule_set.rb | 39 +++++++++--------------- test/test_css_parser_media_types.rb | 5 +++ test/test_helper.rb | 16 ++++++++++ test/test_rule_set_creating_shorthand.rb | 8 +++++ 7 files changed, 52 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52e1cbe..57632be 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/lib/css_parser.rb b/lib/css_parser.rb index 62e87da..97f4f47 100644 --- a/lib/css_parser.rb +++ b/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? diff --git a/lib/css_parser/regexps.rb b/lib/css_parser/regexps.rb index c150785..b8e823e 100644 --- a/lib/css_parser/regexps.rb +++ b/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' diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index cff4bd7..d38b1fa 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -280,7 +280,7 @@ def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, sp end end - @selectors = [] + @selectors = selectors ? parse_selectors!(selectors) : [] @specificity = specificity unless offset.nil? == filename.nil? @@ -290,7 +290,6 @@ def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, sp @offset = offset @filename = filename - parse_selectors!(selectors) if selectors parse_declarations!(block) end @@ -403,14 +402,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 +427,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 +521,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 +583,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 +637,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 +684,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 diff --git a/test/test_css_parser_media_types.rb b/test/test_css_parser_media_types.rb index 87e00e9..c5e2616 100644 --- a/test/test_css_parser_media_types.rb +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 6411176..9f38b97 100644 --- a/test/test_helper.rb +++ b/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(2) do + before = GC.stat(:total_allocated_objects) + yield + GC.stat(:total_allocated_objects) - before + end.min + ensure + GC.enable + end +end diff --git a/test/test_rule_set_creating_shorthand.rb b/test/test_rule_set_creating_shorthand.rb index 63262d0..8af4531 100644 --- a/test/test_rule_set_creating_shorthand.rb +++ b/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) From 6eed0f0e3c88862c5a81005bbecfcd6be79253da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Joubert?= Date: Mon, 26 Oct 2026 12:49:44 +0300 Subject: [PATCH 2/2] Keep RuleSet#initialize ordering, take the minimum of 3 allocation runs Parsing selectors before specificity, offset and filename were set changed what a subclass overriding parse_selectors! could see, and which error bad arguments raised, for a single saved array. GC.stat counts allocations of every thread, so the helper keeps the minimum of three runs. Co-Authored-By: Claude Opus 5.5 --- lib/css_parser/rule_set.rb | 3 ++- test/test_helper.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index d38b1fa..375f871 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -280,7 +280,7 @@ def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, sp end end - @selectors = selectors ? parse_selectors!(selectors) : [] + @selectors = [] @specificity = specificity unless offset.nil? == filename.nil? @@ -290,6 +290,7 @@ def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, sp @offset = offset @filename = filename + parse_selectors!(selectors) if selectors parse_declarations!(block) end diff --git a/test/test_helper.rb b/test/test_helper.rb index 9f38b97..c39b380 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,7 +14,7 @@ def allocations yield GC.disable - Array.new(2) do + Array.new(3) do before = GC.stat(:total_allocated_objects) yield GC.stat(:total_allocated_objects) - before