steel / sprockets-derailleur

Speed up Manifest::Compile by forking processes
MIT License
79 stars 33 forks source link

Not working for sprockets 3 #25

Open camertron opened 8 years ago

camertron commented 8 years ago

After my pull request got merged, I deployed our app to production and broke the world. A number of issues came up, here are a few of the major ones:

  1. sprockets-derailleur will write .gz assets that are not actually gzipped.
  2. the .sprockets-manifest.json file is not updated with correct asset digests.

Unfortunately I had to remove sprockets-derailleur from our Gemfile and will be seeking a different solution. This is really unfortunate since sprockets-derailleur sped up our asset precompile by ~60%!

Thanks for the gem, but beware of sprockets 3.

camertron commented 8 years ago

Here's what we ended up with (in config/initializers/fast_sprockets.rb):

require 'parallel'

module Sprockets
  module ParallelCompiler
    def compile(*args)
      worker_count = ENV.fetch('SPROCKETS_WORKER_COUNT', 4).to_i
      paths = environment.each_logical_path(*args).to_a

      logger.warn "Precompiling with #{worker_count} workers"

      time = Benchmark.measure do
        Parallel.each(paths, in_processes: worker_count) do |logical_path|
          super(logical_path)
        end
      end

      logger.info "Completed compiling assets (#{time.real.round(2)}s)"
    end
  end

  class Manifest
    prepend ParallelCompiler
  end
end
sbleon commented 7 years ago

I ran into this today and didn't know there was a problem until I deployed to staging and my CSS and JS wouldn't load. A warning about Sprockets 3 at the top of the README would be very nice!

camertron commented 7 years ago

@sbleon yeah, that would be great. Feel like submitting a PR?

Also, it turns out the monkeypatch I described above doesn't work either - I had to delete it. There's something special going on with how the manifest gets updated, and I haven't been able to figure it out.

sbleon commented 7 years ago

Yeah, I tried your workaround and found that the manifest is replaced by each "job", so only the last file to finish ends up in the manifest. I'll whip up a PR for the warning.

PikachuEXE commented 7 years ago

Looking at sprockets 3.x https://github.com/rails/sprockets/blob/3.x/lib/sprockets/manifest.rb#L176

It seems the filenames is returned Maybe it will work if we collect the files names in Parallel.each?

PikachuEXE commented 7 years ago

Also concurrent compile might be built-in for 4.x https://github.com/rails/sprockets/blob/master/lib/sprockets/manifest.rb#L163

sbleon commented 7 years ago

@PikachuEXE concurrency isn't touted as a new feature in the Sprockets 4.x upgrade guide. (Although the other new features look great!)

Sprockets 3.x has some code that mentions concurrency as well, but I'm not sure how it works.

wjordan commented 7 years ago
  1. sprockets-derailleur will write .gz assets that are not actually gzipped.

The upstream change causing this issue in 3.0 was sstephenson/sprockets#589, which removed GZip compression from the #write_to method.

GZip file generation was re-introduced in >= 3.5.0 by rails/sprockets#194 within #compile.

  1. the .sprockets-manifest.json file is not updated with correct asset digests.

I wasn't able to reproduce behavior like this in local testing, asset digests in .sprockets-manifest.json seem to update fine. Can anyone confirm this issue (with specific repro steps)?

Also concurrent compile might be built-in for 4.x Sprockets 3.x has some code that mentions concurrency as well, but I'm not sure how it works.

Looks like this upstream concurrency is only being used for Exporters (writing assets and gzipped assets to disk) at the moment, not any other parts of the Sprockets toolchain.

camertron commented 7 years ago

Hey guys, I just couldn't let this go (the speed improvements are too tantalizing!) I ended up finally figuring out how to do a parallel compile and documented it in this gist. You should be able to drop the code into an initializer and see a nice speed increase. We've been using it in production for ~7 months and it's been working flawlessly so far. Is anyone willing to take on the job of incorporating the gist into sprockets-derailleur?

We've also had good success with asset "fingerprinting" which skips precompiling entirely if no assets have changed (see this file in same gist). Could also be a nice feature to add to sprockets-derailleur.

PikachuEXE commented 7 years ago

Thanks @camertron for sharing the gist! I will have a try soon

camertron commented 7 years ago

@PikachuEXE fyi I ended up turning the parallelization gist into a gem called turbo-sprockets-rails4.

PikachuEXE commented 7 years ago

@camertron Using rails 5, can't use :(

camertron commented 7 years ago

@PikachuEXE ah too bad! Despite the name, turbo-sprockets-rails4 should also work with Rails 5, it just hasn't been tested yet. All that really matters is the version of sprockets (~> 3.0). I'll consider releasing a rails5 version :)