Skip to content

Commit

Permalink
Perf: simplify calculation of location
Browse files Browse the repository at this point in the history
- caller is only used to determine file and line number as metadata is
  initialized. There is no need to store it.
- Also, the previous commit reduces the need for analysis of the caller
  stack. We therefore don't need to sling it around within the rspec
  codebase.
- Running the following 5000 times took 2.77 seconds before these last
  two commits, 1:58 after:

    describe "something" do
      it "does something" do
        1.should eq(1)
      end
    end

Please enter the commit message for your changes. Lines starting
  • Loading branch information
dchelimsky committed Oct 14, 2010
1 parent 9c6b6f7 commit f78ff61
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 53 deletions.
2 changes: 0 additions & 2 deletions lib/rspec/core/example_group.rb
Expand Up @@ -42,7 +42,6 @@ def self.define_example_method(name, extra_options={})
module_eval(<<-END_RUBY, __FILE__, __LINE__) module_eval(<<-END_RUBY, __FILE__, __LINE__)
def self.#{name}(desc=nil, options={}, &block) def self.#{name}(desc=nil, options={}, &block)
options.update(:pending => true) unless block options.update(:pending => true) unless block
options.update(:caller => caller)
options.update(#{extra_options.inspect}) options.update(#{extra_options.inspect})
examples << RSpec::Core::Example.new(self, desc, options, block) examples << RSpec::Core::Example.new(self, desc, options, block)
examples.last examples.last
Expand Down Expand Up @@ -111,7 +110,6 @@ def self.describe(*args, &example_group_block)
@_subclass_count += 1 @_subclass_count += 1
args << {} unless args.last.is_a?(Hash) args << {} unless args.last.is_a?(Hash)
args.last.update(:example_group_block => example_group_block) args.last.update(:example_group_block => example_group_block)
args.last.update(:caller => caller)


# TODO 2010-05-05: Because we don't know if const_set is thread-safe # TODO 2010-05-05: Because we don't know if const_set is thread-safe
child = const_set( child = const_set(
Expand Down
1 change: 0 additions & 1 deletion lib/rspec/core/extensions/object.rb
Expand Up @@ -3,7 +3,6 @@ module Core
module ObjectExtensions module ObjectExtensions
def describe(*args, &example_group_block) def describe(*args, &example_group_block)
args << {} unless args.last.is_a?(Hash) args << {} unless args.last.is_a?(Hash)
args.last.update :caller => caller(1)
RSpec::Core::ExampleGroup.describe(*args, &example_group_block) RSpec::Core::ExampleGroup.describe(*args, &example_group_block)
end end
end end
Expand Down
34 changes: 8 additions & 26 deletions lib/rspec/core/metadata.rb
Expand Up @@ -32,9 +32,7 @@ def process(*args)
self[:example_group][:full_description] = full_description_from(*args) self[:example_group][:full_description] = full_description_from(*args)


self[:example_group][:block] = user_metadata.delete(:example_group_block) self[:example_group][:block] = user_metadata.delete(:example_group_block)
self[:example_group][:caller] = user_metadata.delete(:caller) || caller(1) self[:example_group][:file_path], self[:example_group][:line_number] = file_and_line_number_from(caller)
self[:example_group][:file_path] = file_path_from(self[:example_group], user_metadata.delete(:file_path))
self[:example_group][:line_number] = line_number_from(self[:example_group], user_metadata.delete(:line_number))
self[:example_group][:location] = location_from(self[:example_group]) self[:example_group][:location] = location_from(self[:example_group])


update(user_metadata) update(user_metadata)
Expand Down Expand Up @@ -70,11 +68,7 @@ def configure_for_example(description, options)
store(:description, description.to_s) store(:description, description.to_s)
store(:full_description, "#{self[:example_group][:full_description]} #{self[:description]}") store(:full_description, "#{self[:example_group][:full_description]} #{self[:description]}")
store(:execution_result, {}) store(:execution_result, {})
store(:caller, options.delete(:caller)) self[:file_path], self[:line_number] = file_and_line_number_from(caller)
if self[:caller]
store(:file_path, file_path_from(self))
store(:line_number, line_number_from(self))
end
self[:location] = location_from(self) self[:location] = location_from(self)
update(options) update(options)
end end
Expand Down Expand Up @@ -153,31 +147,19 @@ def described_class_from(*args)
end end
end end


def file_path_from(metadata, given_file_path=nil) def file_and_line_number_from(list)
return given_file_path if given_file_path entry = first_caller_from_outside_rspec_from_caller(list)
file = file_and_line_number(metadata)[0] if file_and_line_number(metadata) entry =~ /(.+?):(\d+)(|:\d+)/
file.strip if file return [$1, $2.to_i]
end end


def line_number_from(metadata, given_line_number=nil) def first_caller_from_outside_rspec_from_caller(list)
return given_line_number if given_line_number list.detect {|l| l !~ /\/lib\/rspec\/core/}
line_number = file_and_line_number(metadata)[1] if file_and_line_number(metadata)
line_number && line_number.to_i
end end


def location_from(metadata) def location_from(metadata)
"#{metadata[:file_path]}:#{metadata[:line_number]}" "#{metadata[:file_path]}:#{metadata[:line_number]}"
end end

def file_and_line_number(metadata)
entry = first_caller_from_outside_rspec(metadata)
entry && entry.match(/(.+?):(\d+)(|:\d+)/)[1..2]
end

def first_caller_from_outside_rspec(metadata)
metadata[:caller].detect {|l| l !~ /\/lib\/rspec\/core/}
end

end end
end end
end end
6 changes: 0 additions & 6 deletions spec/rspec/core/example_group_spec.rb
Expand Up @@ -186,12 +186,6 @@ module RSpec::Core
ExampleGroup.describe(Object, nil, 'foo' => 'bar') { }.metadata.should include({ "foo" => 'bar' }) ExampleGroup.describe(Object, nil, 'foo' => 'bar') { }.metadata.should include({ "foo" => 'bar' })
end end


it "adds the caller to metadata" do
ExampleGroup.describe(Object) { }.metadata[:example_group][:caller].any? {|f|
f =~ /#{__FILE__}/
}.should be_true
end

it "adds the the file_path to metadata" do it "adds the the file_path to metadata" do
ExampleGroup.describe(Object) { }.metadata[:example_group][:file_path].should == __FILE__ ExampleGroup.describe(Object) { }.metadata[:example_group][:file_path].should == __FILE__
end end
Expand Down
22 changes: 4 additions & 18 deletions spec/rspec/core/metadata_spec.rb
Expand Up @@ -149,22 +149,13 @@ module Core
]) ])
m[:example_group][:file_path].should == __FILE__ m[:example_group][:file_path].should == __FILE__
end end

it "finds the first spec file in the caller array with drive letter" do
m = Metadata.new
m.process(:caller => [ "C:/path/file_spec.rb:#{__LINE__}" ])
m[:example_group][:file_path].should == "C:/path/file_spec.rb"
end
end end


describe "line number" do describe "line number" do
it "finds the line number with the first non-rspec lib file in the backtrace" do it "finds the line number with the first non-rspec lib file in the backtrace" do
m = Metadata.new m = Metadata.new
m.process(:caller => [ m.process({})
"./lib/rspec/core/foo.rb", m[:example_group][:line_number].should == __LINE__ - 1
"#{__FILE__}:#{__LINE__}",
])
m[:example_group][:line_number].should == __LINE__ - 2
end end


it "finds the line number with the first spec file with drive letter" do it "finds the line number with the first spec file with drive letter" do
Expand All @@ -181,10 +172,9 @@ module Core
end end


describe "metadata for example" do describe "metadata for example" do
let(:caller_for_example) { caller(0) }
let(:line_number) { __LINE__ - 1 }
let(:metadata) { Metadata.new.process("group description") } let(:metadata) { Metadata.new.process("group description") }
let(:mfe) { metadata.for_example("example description", {:caller => caller_for_example, :arbitrary => :options}) } let(:mfe) { metadata.for_example("example description", {:arbitrary => :options}) }
let(:line_number) { __LINE__ - 1 }


it "stores the description" do it "stores the description" do
mfe[:description].should == "example description" mfe[:description].should == "example description"
Expand All @@ -198,10 +188,6 @@ module Core
mfe[:execution_result].should == {} mfe[:execution_result].should == {}
end end


it "stores the caller" do
mfe[:caller].should == caller_for_example
end

it "extracts file path from caller" do it "extracts file path from caller" do
mfe[:file_path].should == __FILE__ mfe[:file_path].should == __FILE__
end end
Expand Down

0 comments on commit f78ff61

Please sign in to comment.