Automatically enable gzip compression

[jeffkaufman]

mod_pagespeed turns on gzip. In ngx_pagespeed we should do this too.

[vidluther]

So, in case like ours, where our web heads are running gzip as well, this would just be overkill right? our web heads shouldn't compress, and ngx_pagespeed does?

[igrigorik]

@vidluther I think it makes sense to gzip as early as possible, since that also reduces transfer sizes within your own infrastructure. Hence, yeah, compress at the origin, or in this case, at ngx_pagespeed.

[vidluther]

So, compress on the web server, and then compress at the nginx servers acting as a reverse proxy/load balancer via ngx_pagespeed as well?

So, in the words of Xzibit..

Yo dawg, I compressed your output inside a compressed output?

[igrigorik]

Err.. may have misinterpreted your comment. PageSpeed will have to decompress any incoming data to rewrite it (obviously). Whether that actually makes sense to do - in terms of spending cycles on both ends - depends on the actual size of response, link speed between your reverse proxy and the application generating the markup, etc. There is not necessarily a "one size fits all" here. Having said that, if ngx_pagespeed is running on same server as app server, then yeah, no reason to enable compression in both places, as you'll be burning CPU three times over on compression/decompression.

[jeffkaufman]

(This should probably come with an option "pagespeed EnableGzip off", but gzip-on is a much better default.)

[k0nsl]

I like this idea, Mr. Kaufman.

On Mon 08 Apr 2013 04:19:41 PM CEST, Jeff Kaufman wrote:

(This should probably come with an option "pagespeed EnableGzip off",
but gzip-on is a much better default.)

—
Reply to this email directly or view it on GitHub
#238 (comment).

[jeffkaufman]

What would it take to turn on gzip by default? We need to set gzip to be on, and set gzip types to include many content types.

ngx_http_gzip_conf_t is defined in ngx_http_gzip_filter_module.c, which means that if we want to reach into the nginx configuration we're going to need to duplicate the structure definition. This is risky, because if there are any changes to this structure in the future we amount to randomly setting bytes. (Yikes!)

The alternative to doing this would be to try to use ngx_http_gzip_filter_commands, which has a standard structure. It stores offsetof(ngx_http_gzip_conf_t, enable) and ngx_string("gzip_types") which is what we need. As a check we could verify that they had pointers to ngx_conf_set_flag_slot and ngx_http_types_slot as expected. Any errors here have to be detected runtime, which is unfortunate, but at least we can detect them, and we can bail out without changing anything.

I see how to go from the gzip module's ngx_module_t to the array of ngx_command_t, and then we can pick out the right one by string comparisons on command->name (looking for gzip and gzip_types). We verify by comparing the function pointer for command->set to the function pointers ngx_conf_set_flag_slot and ngx_http_types_slot. If this succeeds we run those functions on the top level (http-block/main) gzip conf, using a fake ngx_conf_t* to pass in the arguments.

Anyone see a better way?

[jeffkaufman]

Thinking more, we can't just enable gzip at the main level. If pagespeed is on for only one server block, we should only turn on gzip for that server block. So at the point where we're deciding what server blocks to apply to we also need to find and modify the gzip configuration for that server block.

[oschaaf]

@jeffkaufman
Possibly useful: ps_init_child_process() [1] loops over all server contexts. To do something similar for this, we would probably need to add another ngx_module_t and make that run after gzip, so we can check both our and gzip's configuration (which should both be available at that point in time).

[1] https://github.com/pagespeed/ngx_pagespeed/blob/master/src/ngx_pagespeed.cc#L2757

[oschaaf]

@keesspoelstra has explored pulling this off at https://github.com/We-Amp/ngx_pagespeed/tree/keesspoelstra-gzip-enable -- looking promising

[peterbowey]

@oschaaf and @keesspoelstra the pull at https://github.com/We-Amp/ngx_pagespeed/tree/keesspoelstra-gzip-enable looks good - but what about an option to disable for locations / servers where assets are already existing as statically gzip'd.

For example: "pagespeed EnableGzip off"

Less CPU waste and optional when required...

[peterbowey]

@oschaaf your recent updates to this via We-Amp/ngx_pagespeed@67737e3 make sense:

Via changes to src/ngx_gzip_setter.cc

+  // If these functions are called it means there is an explicit gzip
+  // configuration. The gzip configuration set by pagespeed is then rollbacked
+  // and pagespeed will stop enabling gzip automatically.

I assume this indicates we do have control over 'automatic gzip' on this pull?

[oschaaf]

@peterbowey "I assume this indicates we do have control over 'automatic gzip' on this pull?"
@keesspoelstra's work on this makes ngx_pagespeed bail out as far as gzip configuration is concerned when explicit gzip configuration is detected. So yes - you have control over it. If you'd add gzip on or gzip off that would work as expected.

[peterbowey]

@oschaaf Just what I needed to know. Thanks...

[jeffkaufman]

Should I look at the automatic gzip code now, or wait for a pull request?

I'm excited about having gzip on by default with ngx_pagespeed.

[oschaaf]

@jeffkaufman Please wait for a pull, hopefully later today.

[peterbowey]

@jeffkaufman @oschaaf The 'automatic gzip code' addition works fine for me. I tested this via ngx_pagespeed trunk-tracking and the code patches @ https://github.com/We-Amp/ngx_pagespeed/compare/keesspoelstra-gzip-enable.

The above code recognizes previous gzip configurations, in which case it indicates via the nginx error log an entry of: "Pagespeed: rollback gzip, explicit configuration".

However, I am not so sure this report feature should log to the nginx 'error logs'.

[keesspoelstra]

@peterbowey this message should does have level info, how are info level messages normally handled in nginx?
I will also add Pagespeed gzip on/off for location control.

A heads up, I am currently rebasing onto trunk-tracking.

[peterbowey]

@keesspoelstra Both such changes you outline above - are most welcome. At the moment, I am just too tired (@ 1:54 AM Australia) to recall the 'info' level handling for nginx - via the above.

Later, 'I can do', but I am sure you will solve both events, mean-while. Thanks for a great addition to ngx_pagespeed.

[peterbowey]

@keesspoelstra The update at We-Amp/ngx_pagespeed@a1ea5d9 works just fine.

However, after some testing, I decided to revert to manual gzip pagespeed + nginx configurations - (above this auto gzip method). Why? I could not see a direct benefit on existing given controls I encapsulate in any nginx block or server. If I had thought ngx_* may benefit - but it now seems unlikely.

I assume the given 'auto' gzip is a reasonable time saver 'short-cut' on nginx setups along with pagespeed? Did I miss any 'other' attributes of same?

[jeffkaufman]

@peterbowey

I assume the given 'auto' gzip is a reasonable time saver 'short-cut' on nginx setups along with pagespeed?

The goal is to change gzip from opt-in to opt-out. If someone installs pagespeed and knows enough to set a specific gzip configuration then yes, this change is just a time-saving shortcut. But many people installing pagespeed just want it to be a module that does everything it can to make their site load faster, and automatically turning on gzip is pretty helpful for that group.

[jeffkaufman]

Fixed by #665