Concurrent asset compile

[wjordan]

This PR extends the existing code that executes Exporters concurrently to execute the asset-compilation process concurrently as well. This allows time-consuming custom Processor steps (in my use-case, an image-optimization preprocessor provided by image_optim_rails) to leverage multiple CPU cores for performance gains in a similar way to how the GZip/Zopfli Exporters currently do.

The sprockets-derailleur gem added multi-process functionality to Sprockets 2. I'd like to land an optimization upstream with proper test coverage so it can evolve alongside Sprockets more directly and benefit all users of this gem, hopefully by default.

I've tried to make the code diff as minimal as possible and maintain the existing enumerable-stream style (which I assume is meant to minimize memory consumption?). A few points worth noting:

1. I removed Concurrent::FixedThreadPool.new(Concurrent.processor_count) and a condition block from #compile, replacing it with a #exporter method that returns :fast or :immediate symbols depending on the environment configuration. This simplifies the concurrent-execution code by using the default Global Thread Pool for concurrent execution (:fast), and the ImmediateExecutor (:immediate) when concurrency is disabled. This simplification isn't strictly necessary for the feature and could be reverted if desired.
2. Related to the previous point, I reused the existing export_concurrent configuration to determine whether to process assets concurrently. My preference was to have one configuration option controlling all concurrency stemming from Manifest#compile rather than two separate options, but this new functionality could be controlled by a separate configuration option if desired.
3. Because the results of #find are streamed to the Enumerable as they become available, the order of the returned assets is no longer consistent based on the input arguments. I needed to add .sort to a test that broke because it implicitly depended on a consistent ordering of the results. I didn't think this would be an issue for most non-test use-cases, but if this is a potential issue it would be possible to retain the original fixed order (at the expense of not being able to stream the results).
4. Unit tests for concurrent asset processing and also for the existing concurrent exporting feature are included. The tests currently run sleep with a small, fixed duration (currently 0.1 sec) to force a context-switch between threads with minimal test duration, but this approach doesn't seem to be reliable across all the platforms being tested (failed the Windows tests on AppVeyor). If anyone knows of a more reliable method of running a multi-threaded test cross-platform please let me know.

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (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.

[matthewd]

I removed Concurrent::FixedThreadPool.new(Concurrent.processor_count) and a condition block from #compile, replacing it with a #exporter method that returns :fast or :immediate symbols depending on the environment configuration. This simplifies the concurrent-execution code by using the default Global Thread Pool for concurrent execution (:fast), and the ImmediateExecutor (:immediate) when concurrency is disabled. This simplification isn't strictly necessary for the feature and could be reverted if desired.

Did you look into why it's currently done that way before deleting it?

[wjordan]

Did you look into why it's currently done that way before deleting it?

Yes, did you have something specific in mind?

The :fast global executor (which provides a FixedThreadPool with Concurrent.processor_count threads) was not mentioned anywhere in #376 where that line of code was originally added, so I assumed this change had not previously been considered. The behavior should be nearly identical to the local, manually-configured FixedThreadPool it replaces, with less code to maintain locally.

As I mentioned, this simplification isn't strictly necessary for the feature and could be reverted if desired.

[schneems]

By default Concurrent ruby executors will spawn a new thread if all threads are busy. If you're exporting a bunch of assets this could result in many many many idle threads being created instead of queuing up work to be performed by a fixed size thread pool. This isn't ideal and will impact performance negatively. I'm not familiar with the :fast behavior. Do you know whether it scales up when all threads are busy or if it uses a fixed thread pool?

[wjordan]

I'm not familiar with the :fast behavior. Do you know whether it scales up when all threads are busy or if it uses a fixed thread pool?

The :fast global executor provides a FixedThreadPool with Concurrent.processor_count threads.

[schneems]

thanks!

[schneems]

I ended up modifying the test to account for sometimes failures on our windows machine. #515 (diff)