Skip to content

Commit

Permalink
Only compile if no compilation is currently already in progress (#139)
Browse files Browse the repository at this point in the history
When rails tests are run in parallel, multiple `webpack` compilations are kicked off at the same time.
This is _really_ resource intensive and slow.
  • Loading branch information
artemave authored Jul 4, 2022
1 parent 3473ce1 commit 243d3f2
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ Changes since last non-beta release.
### Added
- `append_stylesheet_pack_tag` helper. It helps in configuring stylesheet pack names from the view for a route or partials. It is also required for filesystem-based automated Component Registry API on React on Rails gem. [PR 144](https://github.com/shakacode/shakapacker/pull/144) by [pulkitkkr](https://github.com/pulkitkkr).

### Fixed
- Make sure at most one compilation runs at a time (#139).

_Please add entries here for your pull requests that are not yet released._

## [v6.4.1] - June 5, 2022
Expand Down
44 changes: 39 additions & 5 deletions lib/webpacker/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,53 @@ def initialize(webpacker)
end

def compile
if stale?
run_webpack.tap do |success|
after_compile_hook
end
else
unless stale?
logger.debug "Everything's up-to-date. Nothing to do"
return true
end

if compiling?
wait_for_compilation_to_complete
true
else
acquire_ipc_lock do
run_webpack.tap do |success|
after_compile_hook
end
end
end
end

private
attr_reader :webpacker

def acquire_ipc_lock
open_lock_file do |lf|
lf.flock(File::LOCK_EX)
yield if block_given?
end
end

def locked?
open_lock_file do |lf|
lf.flock(File::LOCK_EX | File::LOCK_NB) != 0
end
end

alias compiling? locked?

def wait_for_compilation_to_complete
logger.info "Waiting for the compilation to complete..."
acquire_ipc_lock
end

def open_lock_file
lock_file_name = File.join(Dir.tmpdir, "shakapacker.lock")
File.open(lock_file_name, File::CREAT) do |lf|
return yield lf
end
end

def optionalRubyRunner
bin_webpack_path = config.root_path.join("bin/webpacker")
first_line = File.readlines(bin_webpack_path).first.chomp
Expand Down

1 comment on commit 243d3f2

@allizon
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit is causing us issues with our CI server -- because of some ancient environmental choices, we have to precompile assets on our Jenkins host before deploying the application to VMs for testing. We frequently have multiple compilations going in parallel, but each build needs its own version of the assets for its particular branch to deploy. Yes, it's resource-intensive, but it's what we need with our setup. Having the lockfile in the server's tmp directory, always with the same name, means only one of those builds will succeed and the rest fail (which I get was the point of the commit). They don't wait for compilation, they just....don't compile and the builds move on and deploy without the compiled assets, and tests fail.

The lockfile is also not being automatically removed when the process is completed, at least for us. It certainly seems like it should be. It's not impossible that's on us somehow, but I'm still looking into that.

I've monkey-patched this file in our repo for now to elide the wait_for_compilation_to_complete method so that it won't hold up the builds, and things seem to be running smoothly. But it would be great not to have to maintain that patch forever. I expect we're not the only team to run into a problem like this, so I think this solution could use a bit of tweaking, even if it just means making it configurable whether to use the lock or not. Moving to Shakapacker has been a huge win for us, but this one thing has caused me several days of headaches running it down.

Please sign in to comment.