Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not working for sprockets 3 #25

Open
camertron opened this issue Aug 15, 2016 · 13 comments
Open

Not working for sprockets 3 #25

camertron opened this issue Aug 15, 2016 · 13 comments

Comments

@camertron
Copy link
Contributor

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
Copy link
Contributor Author

camertron commented Aug 16, 2016

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
Copy link
Contributor

sbleon commented Oct 19, 2016

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
Copy link
Contributor Author

@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
Copy link
Contributor

sbleon commented Oct 21, 2016

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
Copy link

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
Copy link

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

@sbleon
Copy link
Contributor

sbleon commented Oct 31, 2016

@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
Copy link

wjordan commented Apr 9, 2017

  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
Copy link
Contributor Author

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
Copy link

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

@camertron
Copy link
Contributor Author

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

@PikachuEXE
Copy link

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

@camertron
Copy link
Contributor Author

@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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants