r111028 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r111027‎ | r111028 | r111029 >
Date:11:02, 9 February 2012
Author:hashar
Status:reverted (Comments)
Tags:
Comment:
(bug 34254) fix gzipped file cache double compressing output

A gzipped file cache is enabled by setting $wgUseGzip and $wgUseFileCache.
To save CPU cycles, HTMLFileCache would print the compressed output as is
but WebStart uses an ob_gzhandler so we end up with a double compression.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.20 (modified) (history)
  • /trunk/phase3/includes/Setup.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Setup.php
——@@ -388,6 +388,12 @@
389389 $wgCanonicalServer = wfExpandUrl( $wgServer, PROTO_HTTP );
390390 }
391391
 392+if ( $wgUseFileCache && $wgUseGzip ) {
 393+ // Bypass any previous output handler to avoid double
 394+ // compression (bug 34254)
 395+ wfResetOutputBuffers();
 396+}
 397+
392398 wfProfileIn( $fname . '-misc1' );
393399
394400 # Raise the memory limit if it's too low
Index: trunk/phase3/RELEASE-NOTES-1.20
——@@ -18,6 +18,8 @@
1919 * (bug 30245) Use the correct way to construct a log page title.
2020 * (bug 34237) Regenerate an empty user_token and save to the database
2121 when we try to set the user's cookies for login.
 22+* (bug 34254) Using a gzipped file cache ($wgUseGzip and $wgUseFileCache)
 23+ compressed the output twice resulting in garbage output (since 1.18)
2224
2325 === API changes in 1.20 ===
2426

Follow-up revisions

RevisionCommit summaryAuthorDate
r111322revert r111028 (attempt to fix bug 34254)...hashar19:33, 12 February 2012

Comments

#Comment by Aaron Schulz (talk | contribs)   18:56, 9 February 2012

What about pages that cannot be cached? This seems to disable output compression for such cases, which increases network traffic. You only want to disable gzipped buffering when you are going to read or save from the cache.

Places that trap output to the cache have calls like ob_start( array( &$cache, 'saveToFileCache' ) );. ob_start() should create a new buffer that is not compressed. FileCacheBase::saveText() will then gzip or not gzip the uncompressed text as needed, so that aspect is OK. Any double-zipping problems will be in FileCacheBase::loadFromFileCache(), as you probably know already. Maybe the code you added in this revision should go there instead.

#Comment by Hashar (talk | contribs)   19:35, 12 February 2012

I have reverted it with r111322 that revision was broken :-/ The way we handle output buffering compression is too confusing for me, I am going to skip that.

Status & tagging log