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

Fix performance regression with empty template rendering #1544

Merged
merged 1 commit into from Feb 1, 2016

Conversation

pixeltrix
Copy link
Contributor

Closes #1537.

The change in 80637fc introduced a performance regression because the template lookup cache was being lost every time an example was run.

This commit fixes the problem by caching the EmptyTemplateResolver instances between example runs in a hash indexed by the path string.

Closes rspec#1537.

The change in 80637fc introduced a performance regression because the
template lookup cache was being lost every time an example was run.

This commit fixes the problem by caching the EmptyTemplateResolver
instances between example runs in a hash indexed by the path string.
@JonRowe
Copy link
Member

JonRowe commented Jan 31, 2016

Can we add a benchmark for this? Before / After? That's what we'd normally do for this sort of thing... :)

@pixeltrix
Copy link
Contributor Author

@JonRowe commit a benchmark test to the repo?

@JonRowe
Copy link
Member

JonRowe commented Jan 31, 2016

I meant to ./benchmarks but that might actually be a bit tricky, I'd be happy with a note referencing why this was necessary.

@pixeltrix
Copy link
Contributor Author

NOTE: all tests carried out using the sample application in #1537

e52281c (v3.4.0)

$ rspec spec/controllers/badgers_controller_spec.rb
........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Finished in 2.07 seconds (files took 2.98 seconds to load)
1000 examples, 0 failures

cf97d15 (v3.4.1)

$ rspec spec/controllers/badgers_controller_spec.rb
........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Finished in 6.09 seconds (files took 4.25 seconds to load)
1000 examples, 0 failures

As you can see the tests take three times as long to run which is due to the extra file stats carried out whilst searching for the template. In 3.4.0 that template search is cached whereas 3.4.1 it isn't because we're creating a new resolver instance for every example run.

Caching the resolver instances gives the following improvement:

pixeltrix/rspec-rails@52522a9

$ rspec spec/controllers/badgers_controller_spec.rb
........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Finished in 1.77 seconds (files took 2.97 seconds to load)
1000 examples, 0 failures

This actually now gives a slightly improved performance over 3.4.0.

JonRowe added a commit that referenced this pull request Feb 1, 2016
Fix performance regression with empty template rendering
@JonRowe JonRowe merged commit ec1bbc6 into rspec:master Feb 1, 2016
@JonRowe
Copy link
Member

JonRowe commented Feb 1, 2016

Thanks :)

JonRowe added a commit that referenced this pull request Feb 1, 2016
JonRowe added a commit that referenced this pull request Feb 1, 2016
Fix performance regression with empty template rendering
@yujinakayama
Copy link
Member

Will we cut a release for this?

@JonRowe
Copy link
Member

JonRowe commented Feb 2, 2016

Sure, why not, done.

@yujinakayama
Copy link
Member

👍

sebjacobs pushed a commit to futurelearn/rspec-rails that referenced this pull request Mar 15, 2019
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.

Performance Regression in 3.4.1
3 participants