Use Concurrent::FixedThreadPool to control compressors threads count - #339
Conversation
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @schneems (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
|
||
| filenames = [] | ||
| concurrent_compressors = [] | ||
| concurrent_compressors = Concurrent::FixedThreadPool.new( Concurrent.processor_count ) |
There was a problem hiding this comment.
Remove the space inside the parentheses
|
Makes sense. Thanks |
|
I daresay you have larger problems if you're outputting 500 compressible assets... but anyway, that's not how that API works. |
|
Can you fix the style issue mentioned by @rafaelfranca. Also if you rebase against master your tests should pass. That should be good enough to get this across the line. Thanks for the PR! |
|
Can you fix the style issue mentioned by @rafaelfranca. Also if you rebase against master your tests should pass. That should be good enough to get this across the line. |
|
A rebase isn't going to fix this; it is Wrong. |
|
@matthewd could you elaborate ? |
|
I also tried this, it hangs. When I try to precompile assets (with Rails), an error shows up: |
|
I modified the code a little bit, and it's working now. It is in #372 but don't mind copying it. |
|
@hansottowirtz Could you try #376 and let me know if it works for you? |
7 participants
When we have heavy compressors for gzip files assets precompile can end up with 500 or more threads,
with this commit we restrict possible thread counts to processor count.