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
3.x super #507
Closed
Closed
3.x super #507
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
28d9de1
perf: regex only on windows
janbiedermann bc5167e
perf: provide self.fast_compress, saves object inst and method_call
janbiedermann 7258906
match tests to regexp only on windows
janbiedermann 2ddd769
perf: to much work for nothing, no more
janbiedermann 6276753
case is faster then .call_ing on a hash
janbiedermann 54056d7
use fast_compress
janbiedermann 05a6c9a
use caches for expensive paths
janbiedermann 7edd51f
fix for test case
janbiedermann 3c666b4
fix
janbiedermann 7f85cd3
fix
janbiedermann 3d03b3f
add check_modified_root
janbiedermann 4a24c4d
use check_modified_root
janbiedermann 83db3e3
increase default cache
janbiedermann dd4d948
Merge branch '3.x_perf' into 3.x_super
janbiedermann b847baf
optimize split_split_file_uri
janbiedermann 6e66d69
Automatically adjust MemoryStore cache size for a FileStore
janbiedermann fc3fcc6
Merge branch '3.x_perf_proper' into 3.x_super
janbiedermann 7432e83
Automatically adjust MemoryStore size for FileStore
janbiedermann 2bbba2d
fix
janbiedermann File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't significantly faster, only 10% on ruby 2.4 and even slower on older versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, interesting, it makes the digest completely disappear from my callgraph,
what os, cpu are did you try on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, if 10% is not significant ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10% improvement looks nice, but if this call takes for instance only 5% of the entire compilation process, than it's only a microoptimization without any significant effect.
I was wondering how fast this digest optimization is, so I tested it on our pipeline (big project) and here're my numbers (spent time in this method, same hw)
ruby 2.4.2 windows 29,4% faster
ruby 2.4.2 linux 17,1% faster
ruby 2.1.9 windows 8,6% slower
ruby 2.1.9 linux 17.6% faster
jruby 9.1.13.0 windows 10,3% slower
jruby 9.1.13.0 linux 11,1% slower
I didn't have time to dig deeper. In my opinion your implementation should be faster, but optimizations are sometimes tricky :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the numbers, terrific :)
With the compilation time going down 10 times, the digest became very well visible in the call graph, therefore i looked at it, because that small improvement became more significant with overall times going down that much.
ruby 2.1.* is obsolete since spring 2017
jruby only supports rails 4, which is, except for severe security issues, out of support too
And if a case is slower on jruby, and if this project really wants to support jruby, this may be a good case for opening a 'performance: case' ticket over at jruby. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I optimized this using refinements, it's even faster than what we ended up with in MRI. But it made it way slower on jruby.
If you're going to do microbenchmarks, please use benchmark-ips as in
Last time we looked at this the hash method was faster than the case method. A big improvement was added when we started using compare with identity https://github.com/rails/sprockets/pull/439/files.