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/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 new file mode 100644 index 0000000..b8167fd --- /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: 1) 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..7d9490b --- /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: 1) 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..279b4ca --- /dev/null +++ b/test/benchmark/remove_benchmark.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +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) + list.remove(tokens) +end diff --git a/test/benchmark/toggle_benchmark.rb b/test/benchmark/toggle_benchmark.rb new file mode 100644 index 0000000..1c7f468 --- /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: 1) do |n| + list = Classlist.new + n.times { |i| list.toggle("c#{i}") } +end