Skip to content

Gzip files on page caching #4124

Closed
wants to merge 1 commit into from

4 participants

@ai
ai commented Dec 21, 2011

Assets pipeline gzips static files, but page cache doesn’t. It breaks my heart, so I present to you this simple patch.

@yaroslav

/cc @drogus

@josevalim
Ruby on Rails member

Nice. Although I believe we should make gzip opt-in though. And maybe allow to pass the compression mechanism as the option: caches_page :show, :gzip => :best_compression mainly because I actually think we should default to a faster compressor to avoid dog pile effect.

@ai
ai commented Dec 22, 2011

I think, that there isn’t need to change compression level. There are only 2 common cases:
1. Cache page expire more than 1 in minute. If we haven’t resources to best compression, we just disable file compression in Rails and use web server to gzip stream content.
2. Cache page expires less than 1 in minute. It will be better to use best compression.

Of cource, we can imaginate real optimization geek, who try to find best time/size compression level for each page :). For this guys it will be better to disable build-in gzip and just write simple after filter with they own compression.

So we just make code more difficult for very very rare situations (when we need middle compression level).

But, of cource, it isn’t difficult to add compression levels. But maybe it will be better to set level in global config?

@josevalim
Ruby on Rails member

Why have a global config when we could easily do :gzip => :best_compression? Whatever value we pass in, we could simply do a constant lookup: ZLib.const_get options[:gzip].to_s.upcase.

In any case, if you are not interested, you don't need to implement the feature yourself. :) I can do it later. But could you please make it opt-in at least? And thank you for updating the guides in your pull request!

@ai
ai commented Dec 23, 2011

Patch now allow to set compression level.

Maybe it will be better enable gzip by default, same as Assets Pipeline with default gzip compression.

@josevalim
Ruby on Rails member

Awesome, thanks a lot. About the default, I didn't know that the assets pipeline had gzip on by default. That said, let's get some more feedback so I am not the only one deciding this. /cc @fxn @jeremy

@josevalim josevalim added a commit that closed this pull request Dec 24, 2011
@josevalim josevalim Merge branch 'gzip-index' which contains two features:
1) Adding gzip to pages cache, closes #4124

2) Allow scaffold/model/migration generators to accept a "index" and "uniq"
modifiers, as in: "tracking_id:integer:uniq" in order to generate (unique)
indexes. Some types also accept custom options, for instance, you can
specify the precision and scale for decimals as "price:decimal{7,2}".
This feature closes #2555.
0152fe9
@josevalim josevalim closed this in 0152fe9 Dec 24, 2011
@shaliko
shaliko commented Dec 27, 2011

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.