Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Remove Set #1870

Merged
merged 3 commits into from
Feb 20, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions benchmarks/keys_each_vs_each_key.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require 'benchmark/ips'

small_hash = { :key => true, :more_key => true, :other_key => true }
large_hash = (1...100).inject({}) { |hash, key| hash["key_#{key}"] = true; hash }

Benchmark.ips do |x|
x.report('keys.each with small hash') do
small_hash.keys.each { |value| value == true }
end

x.report('each_key with small hash') do
small_hash.each_key { |value| value == true }
end

x.report('keys.each with large hash') do
large_hash.keys.each { |value| value == true }
end

x.report('each_key with large hash') do
large_hash.each_key { |value| value == true }
end
end

__END__

Calculating -------------------------------------
keys.each with small hash
105.581k i/100ms
each_key with small hash
112.045k i/100ms
keys.each with large hash
7.625k i/100ms
each_key with large hash
6.959k i/100ms
-------------------------------------------------
keys.each with small hash
2.953M (± 3.8%) i/s - 14.781M
each_key with small hash
2.917M (± 4.0%) i/s - 14.678M
keys.each with large hash
79.349k (± 2.5%) i/s - 396.500k
each_key with large hash
72.080k (± 2.1%) i/s - 361.868k
1 change: 1 addition & 0 deletions lib/rspec/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
version
warnings

set
flat_map
filter_manager
dsl
Expand Down
7 changes: 3 additions & 4 deletions lib/rspec/core/configuration_options.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
require 'erb'
require 'shellwords'
require 'set'

module RSpec
module Core
Expand Down Expand Up @@ -53,12 +52,12 @@ def organize_options
end
end

UNFORCED_OPTIONS = [
UNFORCED_OPTIONS = Set.new([
:requires, :profile, :drb, :libs, :files_or_directories_to_run,
:full_description, :full_backtrace, :tty
].to_set
])

UNPROCESSABLE_OPTIONS = [:formatters].to_set
UNPROCESSABLE_OPTIONS = Set.new([:formatters])

def force?(key)
!UNFORCED_OPTIONS.include?(key)
Expand Down
4 changes: 2 additions & 2 deletions lib/rspec/core/formatters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ def built_in_formatter(key)
end

def notifications_for(formatter_class)
formatter_class.ancestors.inject(Set.new) do |notifications, klass|
notifications + Loader.formatters.fetch(klass) { Set.new }
formatter_class.ancestors.inject(::RSpec::Core::Set.new) do |notifications, klass|
notifications.merge Loader.formatters.fetch(klass) { ::RSpec::Core::Set.new }
end
end

Expand Down
1 change: 0 additions & 1 deletion lib/rspec/core/formatters/deprecation_formatter.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
RSpec::Support.require_rspec_core "formatters/helpers"
require 'set'

module RSpec
module Core
Expand Down
1 change: 0 additions & 1 deletion lib/rspec/core/reporter.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require 'set'
module RSpec::Core
# A reporter will send notifications to listeners, usually formatters for the
# spec suite run.
Expand Down
41 changes: 41 additions & 0 deletions lib/rspec/core/set.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
module RSpec
module Core
# @private
#
# We use this to replace `::Set` so we can have the advantage of
# constant time key lookups for unique arrays but without the
# potential to pollute a developers environment with an extra
# piece of the stdlib. This helps to prevent false positive
# builds.
#
class Set
include Enumerable

def initialize(array=[])
@values = {}
merge(array)
end

def <<(key)
@values[key] = true
self
end

def each(&block)
@values.keys.each(&block)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you get around to benchmarking Hash#keys.each vs Hash#each_key?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just now, #keys.each is faster but not by a lot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just now, #keys.each is faster but not by a lot.

Weird. I wonder why each_key even exists then...given that all it does is yield each key I'd expect it to be optimized for that case and be faster than the extra hop of keys.each. Can you commit your benchmark for future reference?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it's law of demeter approved ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was already in the process of comitting

self
end

def include?(key)
@values.key?(key)
end

def merge(values)
values.each do |key|
@values[key] = true
end
self
end
end
end
end
2 changes: 1 addition & 1 deletion spec/rspec/core/formatters/base_text_formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
end

def output_from_running(example_group)
allow(RSpec.configuration).to receive(:loaded_spec_files) { [File.expand_path(__FILE__)].to_set }
allow(RSpec.configuration).to receive(:loaded_spec_files) { RSpec::Core::Set.new([File.expand_path(__FILE__)]) }
example_group.run(reporter)
examples = example_group.examples
send_notification :dump_summary, summary_notification(1, examples, examples, [], 0)
Expand Down
23 changes: 23 additions & 0 deletions spec/rspec/core/set_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
RSpec.describe 'RSpec::Core::Set' do

let(:set) { RSpec::Core::Set.new([1, 2, 3]) }

it 'takes an array of values' do
expect(set).to include(1, 2, 3)
end

it 'can be appended to' do
set << 4
expect(set).to include 4
end

it 'can have more values merged in' do
set.merge([4, 5]).merge([6])
expect(set).to include(4, 5, 6)
end

it 'is enumerable' do
expect(set).to be_an Enumerable
expect { |p| set.each(&p) }.to yield_successive_args(1, 2, 3)
end
end
1 change: 0 additions & 1 deletion spec/rspec/core_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
fake_libs = File.expand_path('../../support/fake_libs', __FILE__)
allowed_loaded_features = [
/optparse\.rb/, # Used by OptionParser.
/set\.rb/, # used in a few places but being removed in #1870.
/rbconfig\.rb/, # loaded by rspec-support for OS detection.
/shellwords\.rb/, # used by ConfigurationOptions and RakeTask.
/stringio/, # Used by BaseFormatter.
Expand Down