-
Notifications
You must be signed in to change notification settings - Fork 791
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
Absolute resolved file path slows precompilation between deploys #59
Comments
Thank you for the issue. Could you tell which version of sprockets are you using? |
We're using I should also mention that the manifest file is shared between releases and is in the compilation target directory. |
FYI I just checked and the situation is the same in |
@rafaelfranca any feedback? or a pointer towards where in the code I should look into creating a PR? thank you! |
I didn't have time to look on it yet, but you must look at |
same problem with 3.2.0 and Rails 4.2.2 |
I've had the same problem with long asset precompile times on deploys. Downgrading sprockets to 2.12.4 fixed it for me. |
Sprockets 3.0.3 does not have this problem, but the issue is severe in sprockets >= 3.1.0 Every capistrano deployment for one of our apps is takes 40 minutes to complete the assets:precompile step when no assets have changed. Running There were a ton of cache keys changed between 3.0.3 and 3.1.0; I haven't yet found a single obvious change that introduced this awful behavior. And I see 4.x has now been merged into master, so I'm not even sure which branch I should be trying to fix? |
I noticed the same problem with sprockets 3.2.0 and ckeditor 4.1.2 precompilation. |
@davidalbert and I ran into this yesterday while investigating a substantial increase in our Heroku deploy times. We did not initially realize that this issue was the cause, but it is: every push to Heroku builds your application in a random directory (e.g. Dave and I created a small example Rails application to reproduce: https://github.com/davidbalbert/test-3.2. If anyone is curious, clone, bundle install, and follow these steps: ~/test-3.2 $ time RAILS_ENV=production bin/rake assets:precompile
# Observe long compile time
~/test-3.2 $ time RAILS_ENV=production bin/rake assets:precompile
# Observe short compile time due to cache present in tmp/cache
~/test-3.2 $ cd .. && mv test-3.2 no-cache && cd no-cache
~/no-cache $ time RAILS_ENV=production bin/rake assets:precompile
# Observe long compile time, even though tmp/cache is in place
# and no assets have changed We began looking into potential fixes, but unfortunately weren't familiar enough with sprockets to make meaningful progress. Please let us know if we can be helpful in contributing to a fix. |
@zachallaun great work sleuthing. perhaps we should involve @hone? |
Thanks for the example app. I have a few things to say A) Sprockets 3 is really complicated. B) I think this is where the problem is coming from sprockets/lib/sprockets/base.rb Lines 53 to 55 in 543a5a2
cache.fetch("file_digest:#{path}:#{stat.mtime.to_i}") do
self.stat_digest(path, stat)
end The
This still doesn't explain why it used to work, and what exactly changed. My current guess is that a cache key is generated with the absolute filename, and stored in sprockets/lib/sprockets/manifest.rb Lines 158 to 163 in 37ebcd1
if File.exist?(target)
logger.debug "Skipping #{target}, already exists"
else
logger.info "Writing #{target}"
asset.write_to target
end When you deploy this example app the first time you see the assets written:
On repeated deploys:
It still takes a long time but doesn't output |
@schneems Thanks for looking further into this! I certainly agree with you on point A. There seem to be many layers of caching going on, and I believe that all of them that rely on a path or a URI are broken. Here are another couple of lines that are likely impacting performance heavily:
I think the cache misses indicated above might be the largest performance issue, as they seem to be caching the result of |
@josh, It looks like you wrote much of the code around the areas we've been discussion. If you have time and are willing, it would be great to hear your opinion on this. |
That first line is never executed in the example you gave me
params.key?(:id) will always be false. If someone could set up a script that exits with a 1 when this takes longer than 10 seconds then maybe we could use git bisect to figure out where the problem was introduced. |
Here's some more info for you. I used git bisect to find the regression with this script: https://gist.github.com/schneems/85f592ba2773761dfcf3 After putting that in a local clone of the sprockets repo and pointing the example app to use a path to the local repo I ran
Which shows that the last "good" commit is: 4a16c90 The commit that introduced the regression is: This change was introduced by @josh in sstephenson#555. Maybe he has some ideas as to the failure mode at play here, it's not obvious to me. |
That script isn't perfect and that bisect was a red herring. |
This fix is targeted at 3.x. I haven't tested 4.x for presence of this problem, but I would be surprised if it didn't exist there as well. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.385542936011916 seconds. DON'T MERGE THIS IN YET it needs a cleanup, some better docs, cause geez the 90% of the implicit existing undocumented behavior here is kinda crazy. Not all tests are passing yet either.
Pretty sure it was 100% the cache keys. I made a WIP pull request here if you want to take a look #89. Still needs some love, but you'll get the gist. |
This fix is targeted at 3.x. I haven't tested 4.x for presence of this problem, but I would be surprised if it didn't exist there as well. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.385542936011916 seconds. DON'T MERGE THIS IN YET it needs a cleanup, some better docs, cause geez the 90% of the implicit existing undocumented behavior here is kinda crazy. Not all tests are passing yet either.
This fix is targeted at 3.x. I haven't tested 4.x for presence of this problem, but I would be surprised if it didn't exist there as well. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.385542936011916 seconds. DON'T MERGE THIS IN YET it needs a cleanup, some better docs, cause geez the 90% of the implicit existing undocumented behavior here is kinda crazy. Not all tests are passing yet either.
This fix is targeted at 3.x. It looks like this problem doesn't exist on sprockets 4.x. Why exactly? I have no clue, I would appreciate independent verification of this. Please open up a new issue and @-mention me if you can reproduce this problem with Sprockets 4.x. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
I've got a working solution at #89. I'm done with cleanup, added docs and a test for current behavior. I also updated the PR description. I tried to reproduce the problem with Sprockets 4.x and couldn't so it looks like this issue is isolated to only 3.x though I have no idea why and would like others to double check me to see if they can reproduce the original issue with sprockets 4.x. |
This fix is targeted at 3.x. This problem may or may not exist on 4.x I've been unable to get 4.x working with a Rails app. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
This fix is targeted at 3.x. This problem may or may not exist on 4.x I've been unable to get 4.x working with a Rails app. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in rails#59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
This fix is targeted at 3.x. This problem may or may not exist on 4.x I've been unable to get 4.x working with a Rails app. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
This fix is targeted at 3.x. This problem may or may not exist on 4.x I've been unable to get 4.x working with a Rails app. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in #59. This change introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
[close #59] Use relative path for cache keys
This fix is targeted at 4.x. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in rails#59 and rails#90 This commit is a introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
Confirmed this fixed our problem. Thanks to everyone for the fix! EDIT: I spoke too soon. Our team had to revert the gem version bump because it appears now that dependent files aren't being recompiled when the dependency changes. We'll post more details (or open a new issue) when we have more information. |
Please open a new issue when you find the information. |
please take a look at #96 |
This fix is targeted at 4.x. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in rails#59 and rails#90 This commit is a introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
This fix is targeted at 4.x. Right now the cache is using absolute paths to generate cache keys, this doesn't work if you end up running `rake assets:precompile` in different directories. Say you've moved your project or are running on a different server with a different absolute path. Or say, on every build on Heroku ever. This was reported in rails#59 and rails#90 This commit is a introduces UnloadedAsset class that is responsible for among other things generating consistent cache keys. Previously when running https://gist.github.com/schneems/85f592ba2773761dfcf3 the output would show that the second build took over 38 seconds to build. With this patch applied it takes around 3.38 seconds. Most of this commit is docs, as the current behavior and use of the cache is not really documented, and extremely difficult to follow in the code. All existing behavior is preserved, all tests pass, an extra test was added to ensure that a project built using a previously built directory will be fast.
Hi, I'm running into an issue with the way Sprockets resolves asset paths and wanted to see if there was a way to work around it, and if not, what part of the code would be best to modify for a PR.
Basically, the fact that Sprockets resolves asset URIs to a full file:///full/path/to/a/file means that if a project directory which has done a precompile gets moved to a new directory, the precompile loses the advantage of the FileCache and manifest, because Sprockets thinks the files aren't the same because their full path doesn't match, so it doesn't use the files in the cache.
Our project happens to have a lot of assets, which means even a precompile with a manifest and a populated cache causes Sprockets to recalculate the hashes for each asset, and it ends up taking 15-20 minutes whereas in Rails 3 with
turbo-sprockets-rails3
, the compile took around 5 minutes.If we run the precompile again without changing the parent directory, it finishes in 10 seconds.
Our Rails 4 app deployment process looks like:
/data/app/releases/1
,/data/app/releases/2
,/data/app/releases/3
, etc/data/app/current -> /data/app/releases/3
/data/app/releases/3/public/assets -> /data/app/shared/assets
,/data/app/releases/2/public/assets -> /data/app/shared/assets
, etc./data/app/releases/3/tmp/cache -> /data/app/shared/cache
,/data/app/releases/2/tmp/cache-> /data/app/shared/cache
, etc./data/app/releases/4
rake assets:precompile
task in the new release directorySo I guess my questions are:
I thought perhaps this wasn't a possibility because of the fact that multiple asset paths could share the same relative paths, but the fact that this was achieved in Rails 3 with
turbo-sprockets-rails3
suggests it's possible.Thanks!
The text was updated successfully, but these errors were encountered: