Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow assets to be fetched without processing, in order to generate a digest of their sources #367

Closed
wants to merge 1 commit into from

6 participants

@ndbroadbent

This feature lets us determine if an asset's sources have changed. If not, then we don't need to recompile and recompress that asset.

If :process => false is passed as an option to find_asset, the only processors that are run are Sprockets Directives and ERB. Other processors, such as coffeescript and compression, are removed from the chain. This lets us quickly concatenate the raw dependencies and calculate the digest of the result. We can compare this digest on subsequent runs to determine if the file has changed. I've added a new UnprocessedAsset class, which is basically just ProcessedAsset minus processors. UnprocessedAsset and ProcessedAsset both inherit from AssetWithDependencies.

I haven't touched the Sprockets manifest generation code, so please let me know if you want me to update that to include these source digests.

This PR is a dependency for the changes at sprockets-rails PR #21, which speeds up the Rails asset pipeline for unchanged assets.

Thanks for your time!

@ndbroadbent ndbroadbent referenced this pull request in capistrano/capistrano
Closed

built-in rails asset compile: skip if no changes to assets? #227

@ndbroadbent ndbroadbent Allow assets to be concatenated unprocessed to generate a digest of t…
…heir sources.

This lets us determine if an asset's sources have changed.
If not, then we don't need to recompile and recompress that asset.
If :process => false is passed as an option, the only processors
that are run are Sprockets Directives and ERB.
Other processors, such as coffeescript and compression, are removed.
a8e4032
@fbernier

+1. Just fell on a bug in our production environment where find_asset would throw an error with uglifier not being found when looking for a .js file. this pull request would allow us to use find_asset('myasset.js', :process => false) with more speed and no error.

@ndbroadbent

That's a very interesting use case, thanks for mentioning it! I'm glad this feature could have uses outside of just speeding up asset compilation. Hope someone will be able to review it soon :)

@ndbroadbent

@fbernier, have you tested that this pull request does get rid of your uglifier error? If uglifier is still being referenced too early, I would be happy to look at adding some code to handle the exception if :process is false.

@ndbroadbent ndbroadbent closed this
@ndbroadbent ndbroadbent reopened this
@fbernier

No, I have not tested it =/. I assumed it would fix my problem, but it may need some more tweaking as you mention. I Will try to test it out quite soon, but for now I have used workarounds since I could not wait for this to be merged in.

@josh

This feature lets us determine if an asset's sources have changed. If not, then we don't need to recompile and recompress that asset.

Is already a core feature of sprockets.

All the ProcessedAsset stuff is on the way out. The future source map path is going to kill all the different asset types.

So this isn't the direction things are moving.

@josh josh closed this
@guilleiguaran

This feature lets us determine if an asset's sources have changed. If not, then we don't need to recompile and recompress that asset.

I could be wrong, but I think this can be achieved without change anything in sprockets and just skipping entirely undigested assets compiling (done already in josh's sprockets-rails, I'm porting it in main sprockets-rails) and setting the assets cache property to avoid sharing same assets cache between different environments (done in rails master), right?

@ndbroadbent

@josh, I agree, your source maps feature looks awesome! I noticed that before, and would have definitely used it if it was merged in, so I'm looking forward to that. My sprockets-rails pull request will only need some minor changes to work with source maps.

I don't think this is already a core feature of sprockets, unless you were talking about your source maps work. As far as I can tell, there was no way to build a list of asset dependencies without processing and compressing the final asset. Please let me know if there's a way to do that already, and I'll update the sprockets-rails pull request.

@guilleiguaran - I agree that skipping non-digest assets would be better, and I don't really mind either way. I'll probably just add some images to public/ when I need to have a permanent link for email newsletters and logos.
I'm not sure what you mean about not sharing the assets cache between environments. Will that solve the problem of not recompiling assets that haven't changed?

@josh

So its clear, Sprockets 3.x (with source maps) will have NO "non-digest" support at all. The server will only route digested paths. Even dev mode will generate digests.

I'll probably just add some images to public/ when I need to have a permanent link for email newsletters and logos.

YES, do this. app/assets for dynamic stuff and public/ for purely static non-managed assets.

@ndbroadbent

@guilleiguaran - Thanks for the mention in your blog post :) Now I understand what you mean about sharing the same assets cache between different environments. It looks like the cache will detect changes in dependencies by checking the modification times of files, but that will miss a few cases where recompiling is required.

There are cases where asset processors handle their own internal 'require directives', so sprockets will only see one cached asset even if that asset imports other files. It won't be able to detect when imported files have changed, so it won't recompile the asset. An important example is @import statements in SASS and Less assets. I'm handling this in my gem by recursively replacing those imports with the contents of the imported files.

I'm also running the ERB processor every time, because you can do things like process a directory of icons into CSS with data-uri background images. If you add new icons to the directory, the 'source digest' of the asset would change, but the actual asset file would remain unmodified, so it won't be recompiled. Most assets won't use ERB, and it's pretty fast most of the time, so I think it's important to handle those cases.

What about improving the caching in Sprockets to handle these cases by using source digests instead of modification times? Then, the development environment would also get the speed improvements.

@jrochkind

Taking non-digest-named asset support out of sprockets-rails is going to be a huge pain for some use cases -- presumably those same use cases that had the non-digest-named version in there in the first place, no?

I'm not sure what you mean by 'static assets' -- any asset can change with future versions of the app/gem. One reason to put assets in the pipeline is to get digesting, sure. But another reason is to get the compilation -- sass, coffeescript, minification, building a single asset file from multiple source files. Sometimes you need compilation but need to reveal the lastest version of the asset with a non-digested-name. My cases involve javascript "widgets" that need to be referenced by third-parties at a static location. My case is probably not common, but there must be some reason they were there in the first place, right? What were the use cases that justified the non-digested-name output in the first place, and are they no longer valid/common?

If I have an asset that is compiled (sass, coffeescript, etc), but does need to be available at a persistent location -- you're saying every time it changes, I should move a copy of it myself over to ./public and check it into source repo? Why is this a good idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 28, 2012
  1. @ndbroadbent

    Allow assets to be concatenated unprocessed to generate a digest of t…

    ndbroadbent authored
    …heir sources.
    
    This lets us determine if an asset's sources have changed.
    If not, then we don't need to recompile and recompress that asset.
    If :process => false is passed as an option, the only processors
    that are run are Sprockets Directives and ERB.
    Other processors, such as coffeescript and compression, are removed.
This page is out of date. Refresh to see the latest.
View
2  lib/sprockets/asset.rb
@@ -13,6 +13,8 @@ def self.from_hash(environment, hash)
BundledAsset
when 'ProcessedAsset'
ProcessedAsset
+ when 'UnprocessedAsset'
+ UnprocessedAsset
when 'StaticAsset'
StaticAsset
else
View
2  lib/sprockets/asset_attributes.rb
@@ -34,7 +34,7 @@ def search_paths
# Reverse guess logical path for fully expanded path.
#
# This has some known issues. For an example if a file is
- # shaddowed in the path, but is required relatively, its logical
+ # shadowed in the path, but is required relatively, its logical
# path will be incorrect.
def logical_path
if root_path = environment.paths.detect { |path| pathname.to_s[path] }
View
139 lib/sprockets/asset_with_dependencies.rb
@@ -0,0 +1,139 @@
+require 'sprockets/asset'
+require 'sprockets/utils'
+
+module Sprockets
+ # `AssetWithDependencies` is the base class for `ProcessedAsset` and `UnprocessedAsset`.
+ class AssetWithDependencies < Asset
+
+ # :dependency_digest is used internally to check equality
+ attr_reader :dependency_digest, :source
+
+
+ # Initialize asset from serialized hash
+ def init_with(environment, coder, asset_options = {})
+ asset_options[:bundle] = false
+
+ super(environment, coder)
+
+ @source = coder['source']
+ @dependency_digest = coder['dependency_digest']
+
+ @required_assets = coder['required_paths'].map { |p|
+ p = expand_root_path(p)
+
+ unless environment.paths.detect { |path| p[path] }
+ raise UnserializeError, "#{p} isn't in paths"
+ end
+
+ p == pathname.to_s ? self : environment.find_asset(p, asset_options)
+ }
+ @dependency_paths = coder['dependency_paths'].map { |h|
+ DependencyFile.new(expand_root_path(h['path']), h['mtime'], h['digest'])
+ }
+ end
+
+ # Serialize custom attributes.
+ def encode_with(coder)
+ super
+
+ coder['source'] = source
+ coder['dependency_digest'] = dependency_digest
+
+ coder['required_paths'] = required_assets.map { |a|
+ relativize_root_path(a.pathname).to_s
+ }
+ coder['dependency_paths'] = dependency_paths.map { |d|
+ { 'path' => relativize_root_path(d.pathname).to_s,
+ 'mtime' => d.mtime.iso8601,
+ 'digest' => d.digest }
+ }
+ end
+
+ # Checks if Asset is stale by comparing the actual mtime and
+ # digest to the inmemory model.
+ def fresh?(environment)
+ # Check freshness of all declared dependencies
+ @dependency_paths.all? { |dep| dependency_fresh?(environment, dep) }
+ end
+
+ protected
+ class DependencyFile < Struct.new(:pathname, :mtime, :digest)
+ def initialize(pathname, mtime, digest)
+ pathname = Pathname.new(pathname) unless pathname.is_a?(Pathname)
+ mtime = Time.parse(mtime) if mtime.is_a?(String)
+ super
+ end
+
+ def eql?(other)
+ other.is_a?(DependencyFile) &&
+ pathname.eql?(other.pathname) &&
+ mtime.eql?(other.mtime) &&
+ digest.eql?(other.digest)
+ end
+
+ def hash
+ pathname.to_s.hash
+ end
+ end
+
+ private
+ def build_required_assets(environment, context, asset_options = {})
+ required_paths = context._required_paths + [pathname.to_s]
+ @required_assets = resolve_dependencies(environment, required_paths, asset_options) -
+ resolve_dependencies(environment, context._stubbed_assets.to_a, asset_options)
+ end
+
+ def resolve_dependencies(environment, paths, asset_options = {})
+ asset_options[:bundle] = false
+ assets = []
+ cache = {}
+
+ paths.each do |path|
+ if path == self.pathname.to_s
+ unless cache[self]
+ cache[self] = true
+ assets << self
+ end
+ elsif asset = environment.find_asset(path, asset_options)
+ asset.required_assets.each do |asset_dependency|
+ unless cache[asset_dependency]
+ cache[asset_dependency] = true
+ assets << asset_dependency
+ end
+ end
+ end
+ end
+
+ assets
+ end
+
+ def build_dependency_paths(environment, context, asset_options = {})
+ asset_options[:bundle] = false
+ dependency_paths = {}
+
+ context._dependency_paths.each do |path|
+ dep = DependencyFile.new(path, environment.stat(path).mtime, environment.file_digest(path).hexdigest)
+ dependency_paths[dep] = true
+ end
+
+ context._dependency_assets.each do |path|
+ if path == self.pathname.to_s
+ dep = DependencyFile.new(pathname, environment.stat(path).mtime, environment.file_digest(path).hexdigest)
+ dependency_paths[dep] = true
+ elsif asset = environment.find_asset(path, asset_options)
+ asset.dependency_paths.each do |d|
+ dependency_paths[d] = true
+ end
+ end
+ end
+
+ @dependency_paths = dependency_paths.keys
+ end
+
+ def compute_dependency_digest(environment)
+ required_assets.inject(environment.digest) { |digest, asset|
+ digest.update asset.digest
+ }.hexdigest
+ end
+ end
+end
View
12 lib/sprockets/base.rb
@@ -3,6 +3,7 @@
require 'sprockets/caching'
require 'sprockets/errors'
require 'sprockets/processed_asset'
+require 'sprockets/unprocessed_asset'
require 'sprockets/server'
require 'sprockets/static_asset'
require 'multi_json'
@@ -364,10 +365,14 @@ def build_asset(logical_path, pathname, options)
if attributes_for(pathname).processors.any?
if options[:bundle] == false
circular_call_protection(pathname.to_s) do
- ProcessedAsset.new(index, logical_path, pathname)
+ if options[:process] == false
+ UnprocessedAsset.new(index, logical_path, pathname)
+ else
+ ProcessedAsset.new(index, logical_path, pathname)
+ end
end
else
- BundledAsset.new(index, logical_path, pathname)
+ BundledAsset.new(index, logical_path, pathname, options)
end
else
StaticAsset.new(index, logical_path, pathname)
@@ -375,7 +380,8 @@ def build_asset(logical_path, pathname, options)
end
def cache_key_for(path, options)
- "#{path}:#{options[:bundle] ? '1' : '0'}"
+ options[:process] = true unless options.key?(:process)
+ "#{path}:#{options[:bundle] ? '1' : '0'}:#{options[:process] ? '1' : '0'}"
end
def circular_call_protection(path)
View
33 lib/sprockets/bundled_asset.rb
@@ -10,22 +10,25 @@ module Sprockets
class BundledAsset < Asset
attr_reader :source
- def initialize(environment, logical_path, pathname)
+ def initialize(environment, logical_path, pathname, options = {})
super(environment, logical_path, pathname)
+ @process = options.fetch(:process, true)
- @processed_asset = environment.find_asset(pathname, :bundle => false)
- @required_assets = @processed_asset.required_assets
- @dependency_paths = @processed_asset.dependency_paths
+ @asset = environment.find_asset(pathname, :bundle => false, :process => @process)
+ @required_assets = @asset.required_assets
+ @dependency_paths = @asset.dependency_paths
@source = ""
# Explode Asset into parts and gather the dependency bodies
to_a.each { |dependency| @source << dependency.to_s }
- # Run bundle processors on concatenated source
- context = environment.context_class.new(environment, logical_path, pathname)
- @source = context.evaluate(pathname, :data => @source,
- :processors => environment.bundle_processors(content_type))
+ if @process
+ # Run bundle processors on concatenated source
+ context = environment.context_class.new(environment, logical_path, pathname)
+ @source = context.evaluate(pathname, :data => @source,
+ :processors => environment.bundle_processors(content_type))
+ end
@mtime = (to_a + @dependency_paths).map(&:mtime).max
@length = Rack::Utils.bytesize(source)
@@ -36,10 +39,10 @@ def initialize(environment, logical_path, pathname)
def init_with(environment, coder)
super
- @processed_asset = environment.find_asset(pathname, :bundle => false)
- @required_assets = @processed_asset.required_assets
+ @asset = environment.find_asset(pathname, :bundle => false)
+ @required_assets = @asset.required_assets
- if @processed_asset.dependency_digest != coder['required_assets_digest']
+ if @asset.dependency_digest != coder['required_assets_digest']
raise UnserializeError, "processed asset belongs to a stale environment"
end
@@ -51,19 +54,19 @@ def encode_with(coder)
super
coder['source'] = source
- coder['required_assets_digest'] = @processed_asset.dependency_digest
+ coder['required_assets_digest'] = @asset.dependency_digest
end
# Get asset's own processed contents. Excludes any of its required
# dependencies but does run any processors or engines on the
# original file.
def body
- @processed_asset.source
+ @asset.source
end
# Return an `Array` of `Asset` files that are declared dependencies.
def dependencies
- to_a.reject { |a| a.eql?(@processed_asset) }
+ to_a.reject { |a| a.eql?(@asset) }
end
# Expand asset into an `Array` of parts.
@@ -74,7 +77,7 @@ def to_a
# Checks if Asset is stale by comparing the actual mtime and
# digest to the inmemory model.
def fresh?(environment)
- @processed_asset.fresh?(environment)
+ @asset.fresh?(environment)
end
end
end
View
5 lib/sprockets/environment.rb
@@ -9,7 +9,7 @@
module Sprockets
class Environment < Base
- # `Environment` should initialized with your application's root
+ # `Environment` should be initialized with your application's root
# directory. This should be the same as your Rails or Rack root.
#
# env = Environment.new(Rails.root)
@@ -67,8 +67,9 @@ def index
# Cache `find_asset` calls
def find_asset(path, options = {})
options[:bundle] = true unless options.key?(:bundle)
+ options[:process] = true unless options.key?(:process)
- # Ensure inmemory cached assets are still fresh on every lookup
+ # Ensure in-memory cached assets are still fresh on every lookup
if (asset = @assets[cache_key_for(path, options)]) && asset.fresh?(self)
asset
elsif asset = index.find_asset(path, options)
View
4 lib/sprockets/index.rb
@@ -54,7 +54,9 @@ def file_digest(pathname)
# Cache `find_asset` calls
def find_asset(path, options = {})
- options[:bundle] = true unless options.key?(:bundle)
+ options[:bundle] = true unless options.key?(:bundle)
+ options[:process] = true unless options.key?(:process)
+
if asset = @assets[cache_key_for(path, options)]
asset
elsif asset = super
View
132 lib/sprockets/processed_asset.rb
@@ -1,8 +1,7 @@
-require 'sprockets/asset'
-require 'sprockets/utils'
+require 'sprockets/asset_with_dependencies'
module Sprockets
- class ProcessedAsset < Asset
+ class ProcessedAsset < AssetWithDependencies
def initialize(environment, logical_path, pathname)
super
@@ -21,132 +20,5 @@ def initialize(environment, logical_path, pathname)
elapsed_time = ((Time.now.to_f - start_time) * 1000).to_i
environment.logger.debug "Compiled #{logical_path} (#{elapsed_time}ms) (pid #{Process.pid})"
end
-
- # Interal: Used to check equality
- attr_reader :dependency_digest
-
- attr_reader :source
-
- # Initialize `BundledAsset` from serialized `Hash`.
- def init_with(environment, coder)
- super
-
- @source = coder['source']
- @dependency_digest = coder['dependency_digest']
-
- @required_assets = coder['required_paths'].map { |p|
- p = expand_root_path(p)
-
- unless environment.paths.detect { |path| p[path] }
- raise UnserializeError, "#{p} isn't in paths"
- end
-
- p == pathname.to_s ? self : environment.find_asset(p, :bundle => false)
- }
- @dependency_paths = coder['dependency_paths'].map { |h|
- DependencyFile.new(expand_root_path(h['path']), h['mtime'], h['digest'])
- }
- end
-
- # Serialize custom attributes in `BundledAsset`.
- def encode_with(coder)
- super
-
- coder['source'] = source
- coder['dependency_digest'] = dependency_digest
-
- coder['required_paths'] = required_assets.map { |a|
- relativize_root_path(a.pathname).to_s
- }
- coder['dependency_paths'] = dependency_paths.map { |d|
- { 'path' => relativize_root_path(d.pathname).to_s,
- 'mtime' => d.mtime.iso8601,
- 'digest' => d.digest }
- }
- end
-
- # Checks if Asset is stale by comparing the actual mtime and
- # digest to the inmemory model.
- def fresh?(environment)
- # Check freshness of all declared dependencies
- @dependency_paths.all? { |dep| dependency_fresh?(environment, dep) }
- end
-
- protected
- class DependencyFile < Struct.new(:pathname, :mtime, :digest)
- def initialize(pathname, mtime, digest)
- pathname = Pathname.new(pathname) unless pathname.is_a?(Pathname)
- mtime = Time.parse(mtime) if mtime.is_a?(String)
- super
- end
-
- def eql?(other)
- other.is_a?(DependencyFile) &&
- pathname.eql?(other.pathname) &&
- mtime.eql?(other.mtime) &&
- digest.eql?(other.digest)
- end
-
- def hash
- pathname.to_s.hash
- end
- end
-
- private
- def build_required_assets(environment, context)
- @required_assets = resolve_dependencies(environment, context._required_paths + [pathname.to_s]) -
- resolve_dependencies(environment, context._stubbed_assets.to_a)
- end
-
- def resolve_dependencies(environment, paths)
- assets = []
- cache = {}
-
- paths.each do |path|
- if path == self.pathname.to_s
- unless cache[self]
- cache[self] = true
- assets << self
- end
- elsif asset = environment.find_asset(path, :bundle => false)
- asset.required_assets.each do |asset_dependency|
- unless cache[asset_dependency]
- cache[asset_dependency] = true
- assets << asset_dependency
- end
- end
- end
- end
-
- assets
- end
-
- def build_dependency_paths(environment, context)
- dependency_paths = {}
-
- context._dependency_paths.each do |path|
- dep = DependencyFile.new(path, environment.stat(path).mtime, environment.file_digest(path).hexdigest)
- dependency_paths[dep] = true
- end
-
- context._dependency_assets.each do |path|
- if path == self.pathname.to_s
- dep = DependencyFile.new(pathname, environment.stat(path).mtime, environment.file_digest(path).hexdigest)
- dependency_paths[dep] = true
- elsif asset = environment.find_asset(path, :bundle => false)
- asset.dependency_paths.each do |d|
- dependency_paths[d] = true
- end
- end
- end
-
- @dependency_paths = dependency_paths.keys
- end
-
- def compute_dependency_digest(environment)
- required_assets.inject(environment.digest) { |digest, asset|
- digest.update asset.digest
- }.hexdigest
- end
end
end
View
28 lib/sprockets/unprocessed_asset.rb
@@ -0,0 +1,28 @@
+require 'sprockets/asset_with_dependencies'
+
+module Sprockets
+ class UnprocessedAsset < AssetWithDependencies
+ def initialize(environment, logical_path, pathname)
+ super
+
+ context = environment.context_class.new(environment, logical_path, pathname)
+ attributes = environment.attributes_for(pathname)
+ processors = attributes.processors
+
+ # Remove all engine processors except ERB to return unprocessed source file
+ processors -= (attributes.engines - [Tilt::ERBTemplate])
+
+ @source = context.evaluate(pathname, :processors => processors)
+
+ build_required_assets(environment, context, :process => false)
+ build_dependency_paths(environment, context, :process => false)
+
+ @dependency_digest = compute_dependency_digest(environment)
+ end
+ end
+
+ # Return unprocessed dependencies when initializing asset from serialized hash
+ def init_with(environment, coder)
+ super(environment, coder, :process => false)
+ end
+end
View
6 test/test_caching.rb
@@ -83,7 +83,7 @@ def setup
assert !asset1.equal?(asset2)
end
- test "depedencies are cached" do
+ test "dependencies are cached" do
env = @env1
parent = env['application.js']
@@ -101,7 +101,7 @@ def setup
assert child2.equal?(child1)
end
- test "proccessed and bundled assets are cached separately" do
+ test "processed and bundled assets are cached separately" do
env = @env1
assert_kind_of Sprockets::ProcessedAsset, env.find_asset('gallery.js', :bundle => false)
assert_kind_of Sprockets::BundledAsset, env.find_asset('gallery.js', :bundle => true)
@@ -109,7 +109,7 @@ def setup
assert_kind_of Sprockets::BundledAsset, env.find_asset('gallery.js', :bundle => true)
end
- test "proccessed and bundled assets are cached separately on index" do
+ test "processed and bundled assets are cached separately on index" do
index = @env1.index
assert_kind_of Sprockets::ProcessedAsset, index.find_asset('gallery.js', :bundle => false)
assert_kind_of Sprockets::BundledAsset, index.find_asset('gallery.js', :bundle => true)
View
9 test/test_environment.rb
@@ -516,6 +516,15 @@ def foo; end
@env.unregister_preprocessor('application/javascript', Sprockets::DirectiveProcessor)
assert_equal "// =require \"notfound\"\n;\n", @env["missing_require.js"].to_s
end
+
+ test "returning unprocessed assets to find sources digest" do
+ asset_processed = @env.find_asset 'project.js'
+ asset_unprocessed = @env.find_asset 'project.js', :process => false
+
+ assert asset_unprocessed
+ assert asset_unprocessed != asset_processed
+ assert asset_unprocessed.digest != asset_processed.digest
+ end
end
class TestIndex < Sprockets::TestCase
Something went wrong with that request. Please try again.