[close #233] Allow to register new archivers to make some compressions to asset file by iggant · Pull Request #241 · rails/sprockets

iggant · 2016-02-16T12:15:42Z

This commit allow to register new archiver.

Can easily replace existing gzip implementation to zopfli (as it show in the #233 it can give about 3% space compressing)

Allow to add new archivers such as generate .webp image format for chrome or JPEG-XR for IE, that will help to reduce image size for that browsers)

We can't use it as preprocessor/postprocessor because not all browsers support this format, but will help for precompiled assets, it will compile longer but will reduce overall site size.

rails-bot · 2016-02-16T12:15:43Z

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

rafaelfranca · 2016-02-17T03:31:58Z

Thank you for the pull request. I prefer to reintroduce the encoding negotiation API for this sstephenson#589.

@schneems @matthewd WDYT?

iggant · 2016-02-17T16:10:30Z

@rafaelfranca

But this is completelly different usecases.
Maybe sprockets should support both.

when you make

RAILS_ENV=production bundle exec rake assets:precompile

you don't have encoding string. Therefor in production mode I have

  config.serve_static_files = false
  config.assets.compile = false

I precompile assets and make some addition formats (like webp, jpeg-xr, etc) (it will take some additional space, but space is extremly cheap now),

So users will get the best performance I can provide for their browser.

Why I don't want to compile assets on the fly:

first of all performance reason (even if I use cloudfrond or similar), first user won't get asset immediatly.
second, memory usage, Ideally I expect sprockets only be manifest lookup logic without loading any other logic

sprockets/railtie: 6.0625 MiB (Also required by: sass/rails/railtie)
  sprockets: 2.9453 MiB (Also required by: sprockets/rails/context, sprockets/rails/helper, and 2 others)
    sprockets/manifest: 2.125 MiB (Also required by: sprockets/legacy)
      concurrent: 2.0508 MiB
        concurrent/configuration: 0.7344 MiB (Also required by: concurrent/scheduled_task, concurrent/agent, and 2 others)
          concurrent/delay: 0.5508 MiB (Also required by: concurrent/utility/processor_counter, concurrent)
            concurrent/concern/obligation: 0.4492 MiB (Also required by: concurrent/ivar)
              concurrent/atomic/event: 0.3711 MiB (Also required by: concurrent/executor/immediate_executor, concurrent/atomics, and 3 others)
                concurrent/synchronization: 0.3633 MiB (Also required by: concurrent/executor/abstract_executor_service, concurrent/utility/at_exit, and 28 others)
        concurrent/executors: 0.3281 MiB
    sprockets/environment: 0.3477 MiB
      sprockets/base: 0.3086 MiB (Also required by: sprockets/cached_environment, sprockets/legacy)
  sprockets/rails/context: 2.5273 MiB
    action_view/helpers: 2.5117 MiB (Also required by: action_view/base)
      action_view/helpers/form_helper: 1.6172 MiB (Also required by: action_view/helpers/form_options_helper)
        action_view/helpers/form_tag_helper: 1.3398 MiB
          action_view/helpers/text_helper: 1.2617 MiB
            action_view/helpers/sanitize_helper: 1.207 MiB
              rails-html-sanitizer: 1.207 MiB
                loofah: 1.0508 MiB
                  nokogiri: 0.8398 MiB (Also required by: TOP, fog/xml, and 2 others)
                    nokogiri/xml: 0.332 MiB

rafaelfranca · 2016-02-17T16:30:32Z

That API is not only to compile assets on the fly, we can use the same API to precompile assets. It is the same we have with transformations (coffee -> js, sass -> css, etc.).

rafaelfranca · 2016-02-23T17:10:46Z

+    end
+
+    # Public: Checks if Gzip is enabled.
+    def gzip?


Since this is not coupled to gzip anymore we may need to change these methods. Like archiver_enabled? skip_archiver? and archiver=.

iggant · 2016-02-24T16:28:09Z

@rafaelfranca add new method, but for compability left gzip? method but add deprecation warning note

rafaelfranca · 2016-02-24T16:34:23Z

+    # will register Zlib compression for text file, 
+    # we dont use myme_types here, because different mime_type 
+    # can be compressed by one archiver (text/stylesheets, text/javascript, etc)
+    def register_archiver type, archiver


Put parentheses in the method definitions.

iggant · 2016-03-08T09:05:56Z

@rafaelfranca

sorry for delay,

I've rewrite slightly, and simplify a lit bit.

If archiver is registered it counts as active otherwise we dont know about

hansottowirtz · 2016-09-02T21:59:51Z

I gave this my own spin at #372.

…o gzip anymore

If archiver've registered it counts as active

schneems · 2016-10-10T15:43:43Z

Zopfli and an archiver interface (called exporters) are merged into master. Check out the docs and source if you need to write one. Or ping me with questions on this thread. Thanks for your input. The final solution was a result of everyone's inputs and help.

iggant · 2016-10-15T08:38:49Z

@schneems

can you create beta4 version with this changes

schneems · 2016-10-17T17:00:35Z

Done.

rails-bot assigned rafaelfranca Feb 16, 2016

rafaelfranca reviewed Feb 23, 2016
View reviewed changes

rafaelfranca reviewed Feb 24, 2016
View reviewed changes

iggant mentioned this pull request Apr 24, 2016

Zopfli gzip #233

Closed

iggant added 3 commits September 13, 2016 17:23

Allow to register new archivers to make some compressions to asset file

c9a71d1

Add deprecation warning for gzip? methods since this is not coupled t…

a126ba7

…o gzip anymore

Remove config[:gzip_enabled] options.

35c078e

If archiver've registered it counts as active

schneems closed this Oct 10, 2016

iggant deleted the archiving branch October 15, 2016 08:38

Navigation Menu

Uh oh!

[close #233] Allow to register new archivers to make some compressions to asset file - #241

[close #233] Allow to register new archivers to make some compressions to asset file#241
iggant wants to merge 3 commits into
rails:masterfrom
iggant:archiving

iggant commented Feb 16, 2016

Uh oh!

rails-bot commented Feb 16, 2016

Uh oh!

rafaelfranca commented Feb 17, 2016

Uh oh!

iggant commented Feb 17, 2016

Uh oh!

rafaelfranca commented Feb 17, 2016

Uh oh!

rafaelfranca Feb 23, 2016

Uh oh!

iggant commented Feb 24, 2016

Uh oh!

rafaelfranca Feb 24, 2016

Uh oh!

iggant commented Mar 8, 2016

Uh oh!

hansottowirtz commented Sep 2, 2016

Uh oh!

schneems commented Oct 10, 2016

Uh oh!

iggant commented Oct 15, 2016

Uh oh!

schneems commented Oct 17, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

iggant commented Feb 16, 2016

Uh oh!

rails-bot commented Feb 16, 2016

Uh oh!

rafaelfranca commented Feb 17, 2016

Uh oh!

iggant commented Feb 17, 2016

Uh oh!

rafaelfranca commented Feb 17, 2016

Uh oh!

rafaelfranca Feb 23, 2016

Choose a reason for hiding this comment

Uh oh!

iggant commented Feb 24, 2016

Uh oh!

rafaelfranca Feb 24, 2016

Choose a reason for hiding this comment

Uh oh!

iggant commented Mar 8, 2016

Uh oh!

hansottowirtz commented Sep 2, 2016

Uh oh!

schneems commented Oct 10, 2016

Uh oh!

iggant commented Oct 15, 2016

Uh oh!

schneems commented Oct 17, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants