Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

speed up root_filter #396

Merged
merged 3 commits into from
Aug 21, 2015
Merged

Conversation

raszi
Copy link
Contributor

@raszi raszi commented Jul 6, 2015

Calculating -------------------------------------
         start_with?   103.000  i/100ms
               match    14.000  i/100ms
-------------------------------------------------
         start_with?      1.043k (± 3.9%) i/s -      5.253k
               match    147.419  (± 3.4%) i/s -    742.000

Comparison:
         start_with?:     1042.7 i/s
               match:      147.4 i/s - 7.07x slower

@@ -4,7 +4,7 @@
SimpleCov.profiles.define "root_filter" do
# Exclude all files outside of simplecov root
add_filter do |src|
!(src.filename =~ /^#{Regexp.escape(SimpleCov.root)}/i)
!src.filename.downcase.start_with?(SimpleCov.root.downcase)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm trying to think if there's any chance src.filename might be nil

Also, can you try benchmarking with /^#{Regexp.escape(SimpleCov.root)}/io (the o means it will only compile the interpolated value once), and also maybe \A instead of ^, and also maybe ROOT_FILTER = /^{Regexp.escape(SimpleCov.root)}/i)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am a bit surprised, regex solution is faster, but not because of the extraction and the o modifier but because of the \A change (which of course makes sense), but I did not expect that it will be faster than String#start_with?.

Calculating -------------------------------------
         start_with?   105.000  i/100ms
     match_optimized    14.000  i/100ms
   match_optimized_A   102.435k i/100ms
               match    13.000  i/100ms
-------------------------------------------------
         start_with?      1.110k (± 3.5%) i/s -      5.565k
     match_optimized    147.382  (± 6.1%) i/s -    742.000
   match_optimized_A      2.477M (± 6.6%) i/s -     12.395M
               match    143.284  (± 6.3%) i/s -    715.000

Comparison:
   match_optimized_A:  2477292.0 i/s
         start_with?:     1110.0 i/s - 2231.75x slower
     match_optimized:      147.4 i/s - 16808.64x slower
               match:      143.3 i/s - 17289.33x slower

Copy link
Collaborator

Choose a reason for hiding this comment

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

blog post? That's pretty interesting

Copy link
Collaborator

Choose a reason for hiding this comment

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

mind gisting your benchmark here? or maybe adding it to bench/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bf4
Copy link
Collaborator

bf4 commented Jul 7, 2015

Which rubies have you tested this against? Only MRI? I wonder if that's an artifact of the regex engine (http://www.geocities.jp/kosako3/oniguruma/ ) ref: discussion #396 (comment)

@raszi
Copy link
Contributor Author

raszi commented Jul 7, 2015

I've tested against MRI 2.1.5 only.

@bf4
Copy link
Collaborator

bf4 commented Aug 20, 2015

@xaviershay
Copy link
Collaborator

LGTM

xaviershay added a commit that referenced this pull request Aug 21, 2015
@xaviershay xaviershay merged commit 76ab9c2 into simplecov-ruby:master Aug 21, 2015
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 2, 2015
0.11.0 2015-11-29 ([changes](simplecov-ruby/simplecov@v0.10.0...v0.10.11))
=================

## Enhancements

* Added `SimpleCov.minimum_coverage_by_file` for per-file coverage thresholds. See [#392](simplecov-ruby/simplecov#392) (thanks @ptashman)
* Added `track_files` configuration option to specify a glob to always include in coverage results, whether or not those files are required. See [#422](simplecov-ruby/simplecov#422) (thanks @hugopeixoto)
* Speed up `root_filter` by an order of magnitude. See [#396](simplecov-ruby/simplecov#396) (thanks @raszi)

## Bugfixes

* Fix warning about global variable `$ERROR_INFO`. See [#400](simplecov-ruby/simplecov#400) (thanks @amatsuda)
* Actually recurse upward looking for `.simplecov`, as claimed by the documentation, rather than only the working directory. See [#423](simplecov-ruby/simplecov#423) (thanks @alexdowad)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants