Skip to content

Commit

Permalink
Improve perf of metadata-based shared group inclusion.
Browse files Browse the repository at this point in the history
The call to `RSpec::CallerFilter.first_non_rspec_line`
took most of the time. I originally planned to leverage
rspec/rspec-support#155 but realized we could avoid
calling that entirely by re-using the location from
the group’s metadata.

I re-ran all the benchmarks and updated them. They
look much, much better.
  • Loading branch information
myronmarston committed Jan 15, 2015
1 parent 580bc76 commit a1a3dc4
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 50 deletions.
17 changes: 8 additions & 9 deletions benchmarks/singleton_example_groups/with_config_hooks.rb
Expand Up @@ -10,20 +10,19 @@
BenchmarkHelpers.run_benchmarks

__END__

No match -- without singleton group support
614.53533.8%) i/s - 2.520k
575.25029.0%) i/s - 2.484k
No match -- with singleton group support
555.190 (±21.1%) i/s - 2.496k
503.671 (±21.8%) i/s - 2.250k
Example match -- without singleton group support
574.82131.5%) i/s - 2.491k
544.19125.7%) i/s - 2.160k
Example match -- with singleton group support
436.39125.2%) i/s - 1.872k
413.53822.2%) i/s - 1.715k
Group match -- without singleton group support
544.06331.4%) i/s - 2.112k
517.99828.2%) i/s - 2.058k
Group match -- with singleton group support
457.09818.8%) i/s - 1.961k
431.55415.3%) i/s - 1.960k
Both match -- without singleton group support
554.00430.1%) i/s - 2.255k
525.30625.1%) i/s - 2.107k in 5.556760s
Both match -- with singleton group support
452.83419.7%) i/s - 1.935k
440.28816.6%) i/s - 1.848k
Expand Up @@ -18,18 +18,18 @@
__END__

No match -- without singleton group support
452.01533.8%) i/s - 1.900k
544.39634.0%) i/s - 2.340k
No match -- with singleton group support
464.520 (±31.0%) i/s - 1.887k
451.635 (±31.0%) i/s - 1.935k
Example match -- without singleton group support
476.96134.6%) i/s - 1.978k in 5.340615s
538.78823.8%) i/s - 2.450k
Example match -- with singleton group support
76.17734.1%) i/s - 266.000
342.99022.4%) i/s - 1.440k
Group match -- without singleton group support
364.55428.3%) i/s - 1.372k
509.96926.7%) i/s - 2.070k
Group match -- with singleton group support
281.76124.1%) i/s - 1.200k
405.28420.5%) i/s - 1.518k
Both match -- without singleton group support
281.52127.4%) i/s - 1.188k
513.34424.0%) i/s - 1.927k
Both match -- with singleton group support
297.886 (±18.1%) i/s - 1.288k
406.111 (±18.5%) i/s - 1.760k
16 changes: 8 additions & 8 deletions benchmarks/singleton_example_groups/with_module_inclusions.rb
Expand Up @@ -11,18 +11,18 @@
__END__

No match -- without singleton group support
519.88033.9%) i/s - 2.162k
555.49827.0%) i/s - 2.496k
No match -- with singleton group support
481.33428.5%) i/s - 2.028k
529.82623.0%) i/s - 2.397k in 5.402305s
Example match -- without singleton group support
491.348 (±29.9%) i/s - 2.068k
541.845 (±29.0%) i/s - 2.208k
Example match -- with singleton group support
407.25722.3%) i/s - 1.782k
465.44020.4%) i/s - 2.091k
Group match -- without singleton group support
483.40336.4%) i/s - 1.815k
530.97624.1%) i/s - 2.303k
Group match -- with singleton group support
424.93229.4%) i/s - 1.804k
505.29118.8%) i/s - 2.226k
Both match -- without singleton group support
397.83131.9%) i/s - 1.720k
542.16828.4%) i/s - 2.067k in 5.414905s
Both match -- with singleton group support
424.23325.5%) i/s - 1.720k
503.22627.2%) i/s - 1.880k in 5.621210s
Expand Up @@ -5,18 +5,18 @@
__END__

No match -- without singleton group support
504.86531.9%) i/s - 2.128k
565.19828.8%) i/s - 2.438k
No match -- with singleton group support
463.11526.6%) i/s - 1.998k
539.78118.9%) i/s - 2.496k
Example match -- without singleton group support
472.82531.9%) i/s - 1.938k
539.28728.2%) i/s - 2.450k in 5.555471s
Example match -- with singleton group support
436.53933.9%) i/s - 1.840k
511.57628.1%) i/s - 2.058k
Group match -- without singleton group support
460.64333.4%) i/s - 1.892k
535.29823.2%) i/s - 2.352k
Group match -- with singleton group support
430.33923.2%) i/s - 1.911k
539.45419.1%) i/s - 2.350k
Both match -- without singleton group support
406.71226.6%) i/s - 1.848k
550.93232.1%) i/s - 2.145k in 5.930432s
Both match -- with singleton group support
470.29926.4%) i/s - 1.890k
540.18319.6%) i/s - 2.300k
Expand Up @@ -11,18 +11,18 @@
__END__

No match -- without singleton group support
503.70033.2%) i/s - 2.184k
563.30429.6%) i/s - 2.385k
No match -- with singleton group support
471.01826.8%) i/s - 2.009k
538.73822.3%) i/s - 2.209k
Example match -- without singleton group support
467.85934.8%) i/s - 2.021k in 5.600106s
546.60525.6%) i/s - 2.450k
Example match -- with singleton group support
84.13834.5%) i/s - 296.000 in 5.515586s
421.11123.5%) i/s - 1.845k
Group match -- without singleton group support
384.144 (±27.9%) i/s - 1.560k
536.267 (±27.4%) i/s - 2.050k
Group match -- with singleton group support
349.30127.5%) i/s - 1.288k
508.64417.7%) i/s - 2.268k
Both match -- without singleton group support
388.10025.8%) i/s - 1.702k
538.04727.7%) i/s - 2.067k in 5.431649s
Both match -- with singleton group support
339.31020.3%) i/s - 1.504k
505.38826.7%) i/s - 1.880k in 5.578614s
8 changes: 6 additions & 2 deletions lib/rspec/core/configuration.rb
Expand Up @@ -1158,8 +1158,12 @@ def configure_group_with(group, module_list, application_method)
# Used internally to extend the singleton class of a single example's
# example group instance with modules using `include` and/or `extend`.
def configure_example(example)
@include_modules.items_for(example.metadata).each do |mod|
safe_include(mod, example.example_group_instance.singleton_class)
# We replace the metadata so that SharedExampleGroupModule#included
# has access to the example's metadata[:location].
example.example_group_instance.singleton_class.with_replaced_metadata(example.metadata) do
@include_modules.items_for(example.metadata).each do |mod|
safe_include(mod, example.example_group_instance.singleton_class)
end
end
end

Expand Down
18 changes: 16 additions & 2 deletions lib/rspec/core/example_group.rb
Expand Up @@ -45,7 +45,21 @@ def self.idempotently_define_singleton_method(name, &definition)
# The [Metadata](Metadata) object associated with this group.
# @see Metadata
def self.metadata
@metadata if defined?(@metadata)
@metadata ||= nil
end

# Temporarily replace the provided metadata.
# Intended primarily to allow an example group's singleton class
# to return the metadata of the example that it exists for. This
# is necessary for shared example group inclusion to work properly
# with singleton example groups.
# @private
def self.with_replaced_metadata(meta)
orig_metadata = metadata
@metadata = meta
yield
ensure
@metadata = orig_metadata
end

# @private
Expand Down Expand Up @@ -333,7 +347,7 @@ def self.find_and_eval_shared(label, name, inclusion_location, *args, &customiza
raise ArgumentError, "Could not find shared #{label} #{name.inspect}"
end

SharedExampleGroupInclusionStackFrame.with_frame(name, inclusion_location) do
SharedExampleGroupInclusionStackFrame.with_frame(name, Metadata.relative_path(inclusion_location)) do
module_exec(*args, &shared_block)
module_exec(&customization_block) if customization_block
end
Expand Down
3 changes: 2 additions & 1 deletion lib/rspec/core/shared_example_group.rb
Expand Up @@ -20,7 +20,8 @@ def inspect
# Our definition evaluates the shared group block in the context of the
# including example group.
def included(klass)
SharedExampleGroupInclusionStackFrame.with_frame(@description, RSpec::CallerFilter.first_non_rspec_line) do
inclusion_line = klass.metadata[:location]
SharedExampleGroupInclusionStackFrame.with_frame(@description, inclusion_line) do
klass.class_exec(&@definition)
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/rspec/core/metadata_spec.rb
Expand Up @@ -189,7 +189,7 @@ def metadata_for(*args)

expect(meta[:shared_group_inclusion_backtrace]).to match [ an_object_having_attributes(
:shared_group_name => "some shared behavior",
:inclusion_location => a_string_including("#{__FILE__}:#{line}")
:inclusion_location => a_string_including("#{Metadata.relative_path __FILE__}:#{line}")
) ]
end
end
Expand All @@ -213,7 +213,7 @@ def metadata_for(*args)

expect(meta[:shared_group_inclusion_backtrace]).to match [ an_object_having_attributes(
:shared_group_name => "some shared behavior",
:inclusion_location => a_string_including("#{__FILE__}:#{line}")
:inclusion_location => a_string_including("#{Metadata.relative_path __FILE__}:#{line}")
) ]
end
end
Expand All @@ -239,11 +239,11 @@ def metadata_for(*args)
expect(meta[:shared_group_inclusion_backtrace]).to match [
an_object_having_attributes(
:shared_group_name => "inner",
:inclusion_location => a_string_including("#{__FILE__}:#{inner_line}")
:inclusion_location => a_string_including("#{Metadata.relative_path __FILE__}:#{inner_line}")
),
an_object_having_attributes(
:shared_group_name => "outer",
:inclusion_location => a_string_including("#{__FILE__}:#{outer_line}")
:inclusion_location => a_string_including("#{Metadata.relative_path __FILE__}:#{outer_line}")
),
]
end
Expand Down

0 comments on commit a1a3dc4

Please sign in to comment.