Short circuit entry check #331
Merged
Conversation
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
|
Turns out it's perhaps faster, but not by 400% made a mistake while benchmarking. |
The method `match_path_extname` is expensive. We know that if the name of the file on disk does not start with the same basename that we're looking for then it won't matter what extensions it's got. This check is pretty fast. On the example app https://github.com/nfm/sprockets-3.x-performance-regressions It reduced asset lookup from 15 seconds to 2.6 seconds. It's still not perfect but it's a 400% + speed increase in the case of looking for an asset in a huge directory. Related comment #84 (comment)
schneems
force-pushed
the
schneems/short-circuit-entry-check
branch
from
dbe9794 to
fb35173
Compare
|
With this PR Without this PR Versus Sprockets 2.x |
|
Note that I removed the |
schneems
added a commit
that referenced
this pull request
Jun 28, 2016
Instead of iterating every entry in a given path, we only pass in valid mime types and explicitly check for basenames that match the extensions we are looking for (when extensions are sufficiently small). For example if we are looking for `foo` with mime of `image/png` instead of looking at EVERY entry in the directory instead we will explicitly look for: - foo.png This only works for mime types that have few extensions. It does not scale well for mime types like javascript that would require many many file touches (.coffee, .coffee.erb, .js, .js.erb, .eco, .eco.erb, etc.) even though there may only be a few entries in the directory. When confronted with many extensions (using magic number 3) we go back to the old method of looping through all entries in the directory. This PR makes searching for files with no transforms very fast. For example images in large directories will be much faster as they will only require one lookup instead of looping through all entries. Using the same script from #331 ``` $ be rake sprockets:benchmark Running benchmark... 1500 lookups of the same asset took 2.927082 1500 lookups of 1500 different assets took 1.838495 ``` Which is a substantial speedup. In the future if we could imply the user's preferred extension i.e if a user uses `.coffee.erb` for all JS files and never uses `.eco.js` then we could make this even faster. We could record the file extensions that are used when running in the cache, prefer those when available, when not available fall back to this logic and record the file extension.
schneems
added a commit
that referenced
this pull request
Jun 28, 2016
Instead of iterating every entry in a given path, we only pass in valid mime types and explicitly check for basenames that match the extensions we are looking for (when extensions are sufficiently small). For example if we are looking for `foo` with mime of `image/png` instead of looking at EVERY entry in the directory instead we will explicitly look for: - foo.png This only works for mime types that have few extensions. It does not scale well for mime types like javascript that would require many many file touches (.coffee, .coffee.erb, .js, .js.erb, .eco, .eco.erb, etc.) even though there may only be a few entries in the directory. When confronted with many extensions (using magic number 3) we go back to the old method of looping through all entries in the directory. This PR makes searching for files with no transforms very fast. For example images in large directories will be much faster as they will only require one lookup instead of looping through all entries. Using the same script from #331 ``` $ be rake sprockets:benchmark Running benchmark... 1500 lookups of the same asset took 1.369423 1500 lookups of 1500 different assets took 2.058736 ``` Which is a substantial speedup. In the future if we could imply the user's preferred extension i.e if a user uses `.coffee.erb` for all JS files and never uses `.eco.js` then we could make this even faster. We could record the file extensions that are used when running in the cache, prefer those when available, when not available fall back to this logic and record the file extension.
schneems
added a commit
that referenced
this pull request
Jun 29, 2016
[close #331] Smarter entry lookup
schneems
added a commit
that referenced
this pull request
Jul 1, 2016
With this patch using the benchmark script https://github.com/schneems/sprockets-3.x-performance-regressions: ``` 1500 lookups of the same asset took 2.900005 1500 lookups of 1500 different assets took 2.746331 ``` Without this patch using the benchmark script: ``` 1500 lookups of the same asset took 12.361615 1500 lookups of 1500 different assets took 12.627099 ```
schneems
added a commit
that referenced
this pull request
Jul 1, 2016
With this patch using the benchmark script https://github.com/schneems/sprockets-3.x-performance-regressions: ``` 1500 lookups of the same asset took 2.900005 1500 lookups of 1500 different assets took 2.746331 ``` Without this patch using the benchmark script: ``` 1500 lookups of the same asset took 12.361615 1500 lookups of 1500 different assets took 12.627099 ``` Which is a 4x speed bump.
schneems
added a commit
that referenced
this pull request
Jul 1, 2016
With this patch using the benchmark script https://github.com/schneems/sprockets-3.x-performance-regressions: ``` 1500 lookups of the same asset took 2.900005 1500 lookups of 1500 different assets took 2.746331 ``` Without this patch using the benchmark script: ``` 1500 lookups of the same asset took 12.361615 1500 lookups of 1500 different assets took 12.627099 ``` Which is a 4x speed bump.
schneems
added a commit
that referenced
this pull request
Jul 1, 2016
Backport #331 - Asset lookup 4x Faster in large directories
schneems
added a commit
that referenced
this pull request
Jul 21, 2016
With this patch using the benchmark script https://github.com/schneems/sprockets-3.x-performance-regressions: ``` 1500 lookups of the same asset took 2.900005 1500 lookups of 1500 different assets took 2.746331 ``` Without this patch using the benchmark script: ``` 1500 lookups of the same asset took 12.361615 1500 lookups of 1500 different assets took 12.627099 ``` Which is a 4x speed bump.
schneems
added a commit
that referenced
this pull request
Jul 25, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The method
match_path_extnameis expensive. We know that if the name of the file on disk does not start with the same basename that we're looking for then it won't matter what extensions it's got. This check is pretty fast.On the example app
https://github.com/nfm/sprockets-3.x-performance-regressions
It reduced asset lookup from 15 seconds to 2.6 seconds. It's still not perfect but it's a 400% + speed increase in the case of looking for an asset in a huge directory.
Related comment #84 (comment)