From 9bd64bbdd072c2c9778260c47dd8fafc5428760a Mon Sep 17 00:00:00 2001 From: Jakob Skjerning Date: Thu, 6 Aug 2026 09:28:16 +0200 Subject: [PATCH 1/2] Add a few benchmarks Run them with `rake benchmark`: #add with many distinct tokens n=500 0.0012s n=4000 0.0667s ratio= 54.72 max= 128.00 OK incremental + and render on each step n=300 0.0017s n=2400 0.0852s ratio= 50.26 max= 128.00 OK chained + with a single render at the end n=300 0.0007s n=2400 0.0280s ratio= 38.69 max= 128.00 OK #remove with many distinct tokens n=500 0.0018s n=4000 0.1076s ratio= 59.70 max= 128.00 OK #toggle with many distinct tokens n=500 0.0024s n=4000 0.1329s ratio= 55.74 max= 128.00 OK --- Rakefile | 5 ++ test/benchmark/add_benchmark.rb | 8 +++ test/benchmark/benchmark_helper.rb | 51 +++++++++++++++++++ .../incremental_add_and_render_benchmark.rb | 17 +++++++ test/benchmark/merge_operations_benchmark.rb | 13 +++++ test/benchmark/remove_benchmark.rb | 9 ++++ test/benchmark/toggle_benchmark.rb | 8 +++ 7 files changed, 111 insertions(+) create mode 100644 test/benchmark/add_benchmark.rb create mode 100644 test/benchmark/benchmark_helper.rb create mode 100644 test/benchmark/incremental_add_and_render_benchmark.rb create mode 100644 test/benchmark/merge_operations_benchmark.rb create mode 100644 test/benchmark/remove_benchmark.rb create mode 100644 test/benchmark/toggle_benchmark.rb diff --git a/Rakefile b/Rakefile index 7c88acc..7b87b4f 100644 --- a/Rakefile +++ b/Rakefile @@ -10,4 +10,9 @@ Rake::TestTask.new(:test) do |t| t.test_files = FileList["test/**/test_*.rb"] end +desc "Run performance benchmarks, failing if any operation's runtime is scaling worse than expected" +task :benchmark do + FileList["test/benchmark/*_benchmark.rb"].sort.each { |f| load f } +end + task default: %i[test standard] diff --git a/test/benchmark/add_benchmark.rb b/test/benchmark/add_benchmark.rb new file mode 100644 index 0000000..09e8026 --- /dev/null +++ b/test/benchmark/add_benchmark.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require_relative "benchmark_helper" + +Bench.scale_check("#add with many distinct tokens", small: 500, large: 4000, expected_order: 2) do |n| + list = Classlist.new + list.add((0...n).map { |i| "c#{i}" }) +end diff --git a/test/benchmark/benchmark_helper.rb b/test/benchmark/benchmark_helper.rb new file mode 100644 index 0000000..3e1fb81 --- /dev/null +++ b/test/benchmark/benchmark_helper.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require "benchmark" + +$LOAD_PATH.unshift(File.expand_path("../../lib", __dir__)) + +require "classlist" +require "classlist/add" +require "classlist/remove" +require "classlist/reset" + +# Small harness for catching algorithmic regressions (O(n) -> O(n^2) and +# worse), rather than measuring absolute speed. Absolute timings are too +# noisy across machines/CI runners to assert on directly, but the *ratio* of +# runtime between a small and a large input is a reasonably stable signal: +# if it grows much faster than the algorithm's expected order, something +# regressed. +module Bench + class RegressionError < StandardError; end + + # Times the given block once for `small` and once for `large`, and raises + # RegressionError unless the runtime scaled roughly as expected. + # + # expected_order: the Big-O exponent the operation should scale by, e.g. + # 1 for O(n), 2 for O(n^2). + # tolerance: how much slack to allow over the expected ratio before + # flagging a regression, to absorb machine noise. + def self.scale_check(name, small:, large:, expected_order:, tolerance: 2) + small_time = Benchmark.realtime { yield(small) } + large_time = Benchmark.realtime { yield(large) } + + size_ratio = large.to_f / small + max_allowed_ratio = (size_ratio**expected_order) * tolerance + actual_ratio = small_time.zero? ? 0 : large_time / small_time + + ok = actual_ratio <= max_allowed_ratio + + printf( + "%-42s n=%-5d %8.4fs n=%-5d %8.4fs ratio=%7.2f max=%7.2f %s\n", + name, small, small_time, large, large_time, actual_ratio, max_allowed_ratio, + ok ? "OK" : "FAIL" + ) + + return if ok + + raise RegressionError, + "#{name}: time grew #{actual_ratio.round(2)}x for a #{size_ratio.round(2)}x increase in " \ + "input size (expected at most #{max_allowed_ratio.round(2)}x for an O(n^#{expected_order}) " \ + "operation with #{tolerance}x tolerance)" + end +end diff --git a/test/benchmark/incremental_add_and_render_benchmark.rb b/test/benchmark/incremental_add_and_render_benchmark.rb new file mode 100644 index 0000000..491617d --- /dev/null +++ b/test/benchmark/incremental_add_and_render_benchmark.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require_relative "benchmark_helper" + +# Regression guard for https://github.com/substancelab/classlist/pull/19 - +# in 1.1.1, Classlist#+ started mutating the receiver and pushing an +# operation onto @operations that was never cleared. Every subsequent call +# to #to_s/#to_a/#== re-resolved the *entire* history of operations, turning +# this common "add a class, render, add another, render" pattern from +# roughly O(n^2) into O(n^3). +Bench.scale_check("incremental + and render on each step", small: 300, large: 2400, expected_order: 2) do |n| + list = Classlist.new(["base"]) + n.times do |i| + list += Classlist.new(["c#{i}"]) + list.to_s + end +end diff --git a/test/benchmark/merge_operations_benchmark.rb b/test/benchmark/merge_operations_benchmark.rb new file mode 100644 index 0000000..4025dab --- /dev/null +++ b/test/benchmark/merge_operations_benchmark.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require_relative "benchmark_helper" + +# Chaining a lot of + operations and only rendering once at the end - the +# "build once, render once" pattern that should stay cheap even when the +# incremental render-on-every-step pattern (see +# incremental_add_and_render_benchmark.rb) does not. +Bench.scale_check("chained + with a single render at the end", small: 300, large: 2400, expected_order: 2) do |n| + list = Classlist.new(["base"]) + n.times { |i| list += Classlist.new(["c#{i}"]) } + list.to_s +end diff --git a/test/benchmark/remove_benchmark.rb b/test/benchmark/remove_benchmark.rb new file mode 100644 index 0000000..35fed5a --- /dev/null +++ b/test/benchmark/remove_benchmark.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require_relative "benchmark_helper" + +Bench.scale_check("#remove with many distinct tokens", small: 500, large: 4000, expected_order: 2) do |n| + tokens = (0...n).map { |i| "c#{i}" } + list = Classlist.new(tokens) + list.remove(tokens) +end diff --git a/test/benchmark/toggle_benchmark.rb b/test/benchmark/toggle_benchmark.rb new file mode 100644 index 0000000..76ca3df --- /dev/null +++ b/test/benchmark/toggle_benchmark.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require_relative "benchmark_helper" + +Bench.scale_check("#toggle with many distinct tokens", small: 500, large: 4000, expected_order: 2) do |n| + list = Classlist.new + n.times { |i| list.toggle("c#{i}") } +end From d4a5b85fb83639da6a86a3d41918fb476032ae61 Mon Sep 17 00:00:00 2001 From: Jakob Skjerning Date: Thu, 6 Aug 2026 11:35:03 +0200 Subject: [PATCH 2/2] Improve performance of Classlist operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By maintaining a Set alongside the ordered array of entries we can reduce the time complexity of add, toggle, include?, and replace from O(n·m) to effectively O(m) (amortized O(1) per token). - Added an @entries_set (Set) maintained alongside the ordered @entries array. include?, add, toggle's existence check, and replace's existence checks now use it for O(1) membership lookups instead of an O(n) Array#include? scan. - add and remove update both structures together; replace syncs the set when it swaps a token in place. - Added a reset(tokens) method that replaces all entries and rebuilds the set atomically — needed because Classlist::Reset was previously reaching into the raw array via entries.replace(entries), which would have silently desynced the set. Updated lib/classlist/reset.rb to call original.reset(entries) instead. Impact: add, toggle, include?, replace, and any +-chain building on them went from O(n·m) to effectively O(m) (amortized O(1) per token). Benchmarks confirm it — add at n=4000 dropped from 0.068s to 0.002s, toggle similarly. What's still O(n) per call, and why I left it: remove uses Array#delete, which has to shift elements after removing one — that's inherent to array-backed ordered storage and isn't fixable by adding a set (the set only helps skip delete calls for tokens that were never present). Fixing that would require swapping the whole backing structure (e.g. a hash + linked list), which is a much bigger change than what was asked here. Since add/toggle/chained-+ are now genuinely linear, I lowered their expected_order from 2 to 1 in test/benchmark/ — with the old threshold, a regression back to O(n²) wouldn't have tripped the guard. remove_benchmark.rb stays at order 2 with a comment explaining why. --- lib/classlist.rb | 30 ++++++++++++++------ lib/classlist/reset.rb | 4 +-- test/benchmark/add_benchmark.rb | 2 +- test/benchmark/merge_operations_benchmark.rb | 2 +- test/benchmark/remove_benchmark.rb | 3 ++ test/benchmark/toggle_benchmark.rb | 2 +- 6 files changed, 30 insertions(+), 13 deletions(-) diff --git a/lib/classlist.rb b/lib/classlist.rb index 83e4841..a7f178c 100644 --- a/lib/classlist.rb +++ b/lib/classlist.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "forwardable" +require "set" # standard:disable Lint/RedundantRequireStatement -- needed on Ruby < 3.2 require_relative "classlist/version" @@ -46,9 +47,11 @@ def ==(other) # Adds the given tokens to the list, omitting any that are already present. def add(tokens) - entries = build_entries(tokens) - entries.each do |entry| - self.entries.push(entry) unless self.entries.include?(entry) + build_entries(tokens).each do |entry| + next if @entries_set.include?(entry) + + entries.push(entry) + @entries_set << entry end end @@ -57,12 +60,13 @@ def add_operation(other) end def include?(token) - entries.include?(token) + @entries_set.include?(token) end alias_method :contains, :include? def initialize(entries = []) @entries = build_entries(entries) + @entries_set = @entries.to_set @operations = [] end @@ -87,12 +91,20 @@ def merge(classlist) # Removes the specified tokens from the classlist, ignoring any that are not # present. def remove(tokens) - entries = build_entries(tokens) - entries.each do |entry| - self.entries.delete(entry) + build_entries(tokens).each do |entry| + next unless @entries_set.delete?(entry) + + entries.delete(entry) end end + # Replaces all entries in the classlist with the given tokens. + def reset(tokens) + @entries = build_entries(tokens) + @entries_set = @entries.to_set + @entries + end + # Replaces an existing token with a new token. If the first token doesn't # exist, #replace returns false immediately, without adding the new token to # the token list. @@ -104,6 +116,8 @@ def replace(old_token, new_token) else index = entries.index(old_token) entries[index] = new_token + @entries_set.delete(old_token) + @entries_set << new_token end true @@ -136,7 +150,7 @@ def to_s def toggle(token, force = nil) raise ArgumentError, "The token can not contain whitespace." if token.to_s.include?(" ") - if entries.include?(token) + if include?(token) remove(token) unless force == true result = false else diff --git a/lib/classlist/reset.rb b/lib/classlist/reset.rb index b6fcc2a..3ae65cb 100644 --- a/lib/classlist/reset.rb +++ b/lib/classlist/reset.rb @@ -6,12 +6,12 @@ # classlist when merged. class Classlist::Reset < Classlist::Operation def merge(original) - original.entries.replace(entries) + original.reset(entries) end # #resolve changes the original classlist def resolve(original) - original.entries.replace(entries) + original.reset(entries) super end diff --git a/test/benchmark/add_benchmark.rb b/test/benchmark/add_benchmark.rb index 09e8026..b8167fd 100644 --- a/test/benchmark/add_benchmark.rb +++ b/test/benchmark/add_benchmark.rb @@ -2,7 +2,7 @@ require_relative "benchmark_helper" -Bench.scale_check("#add with many distinct tokens", small: 500, large: 4000, expected_order: 2) do |n| +Bench.scale_check("#add with many distinct tokens", small: 500, large: 4000, expected_order: 1) do |n| list = Classlist.new list.add((0...n).map { |i| "c#{i}" }) end diff --git a/test/benchmark/merge_operations_benchmark.rb b/test/benchmark/merge_operations_benchmark.rb index 4025dab..7d9490b 100644 --- a/test/benchmark/merge_operations_benchmark.rb +++ b/test/benchmark/merge_operations_benchmark.rb @@ -6,7 +6,7 @@ # "build once, render once" pattern that should stay cheap even when the # incremental render-on-every-step pattern (see # incremental_add_and_render_benchmark.rb) does not. -Bench.scale_check("chained + with a single render at the end", small: 300, large: 2400, expected_order: 2) do |n| +Bench.scale_check("chained + with a single render at the end", small: 300, large: 2400, expected_order: 1) do |n| list = Classlist.new(["base"]) n.times { |i| list += Classlist.new(["c#{i}"]) } list.to_s diff --git a/test/benchmark/remove_benchmark.rb b/test/benchmark/remove_benchmark.rb index 35fed5a..279b4ca 100644 --- a/test/benchmark/remove_benchmark.rb +++ b/test/benchmark/remove_benchmark.rb @@ -2,6 +2,9 @@ require_relative "benchmark_helper" +# #remove is still O(n) per call (Array#delete has to shift elements), so +# removing n tokens from an n-entry list is O(n^2) - unlike #add/#toggle, +# which the entries Set made effectively O(1) per call. Bench.scale_check("#remove with many distinct tokens", small: 500, large: 4000, expected_order: 2) do |n| tokens = (0...n).map { |i| "c#{i}" } list = Classlist.new(tokens) diff --git a/test/benchmark/toggle_benchmark.rb b/test/benchmark/toggle_benchmark.rb index 76ca3df..1c7f468 100644 --- a/test/benchmark/toggle_benchmark.rb +++ b/test/benchmark/toggle_benchmark.rb @@ -2,7 +2,7 @@ require_relative "benchmark_helper" -Bench.scale_check("#toggle with many distinct tokens", small: 500, large: 4000, expected_order: 2) do |n| +Bench.scale_check("#toggle with many distinct tokens", small: 500, large: 4000, expected_order: 1) do |n| list = Classlist.new n.times { |i| list.toggle("c#{i}") } end