r108284 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r108283‎ | r108284 | r108285 >
Date:21:49, 6 January 2012
Author:hashar
Status:reverted (Comments)
Tags:
Comment:
Fix formatBitrate behavior on Mac OS X

Language::formatBitrate() uses log10() to makes a long number human readeable.
There is a nasty rounding error on Mac OS X for log10():

log10(pow(10,15)) => gives 15

floor( log10(pow(10,15)) ) => gives 14 (should be 15)

The end result is that pow(10,15) is formatted as 1,000Tbps instead of 1Pbps

log( $foo, 10) does not suffer from this:

php -r 'print floor(log(pow(10,15),10)) ."\n";'

PHP Version used:

$ php -v
PHP 5.3.6 with Suhosin-Patch (cli) (built: Sep 8 2011 19:34:00)
Copyright (c) 1997-2011 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2011 Zend Technologies
with Xdebug v2.1.2, Copyright (c) 2002-2011, by Derick Rethans
$

TEST PLAN:

BEFORE
======

$ php phpunit.php ./languages/LanguageTest.php
PHPUnit 3.6.3 by Sebastian Bergmann.

............................................................... 63 / 170 ( 37%)
............................................................... 126 / 170 ( 74%)
.......................................F....

Time: 2 seconds, Memory: 32.25Mb

There was 1 failure:

1) LanguageTest::testFormatBitrate with data set #5 (1000000000000000, '1Pbps', '1 petabit per second')
formatBitrate('1000000000000000'): 1 petabit per second
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1Pbps'
+'1,000Tbps'

FAILURES!
Tests: 170, Assertions: 174, Failures: 1.


AFTER
=====

PHPUnit 3.6.3 by Sebastian Bergmann.

............................................................... 63 / 170 ( 37%)
............................................................... 126 / 170 ( 74%)
............................................

Time: 1 second, Memory: 32.25Mb

OK (170 tests, 174 assertions)
Modified paths:
  • /trunk/phase3/languages/Language.php (modified) (history)

Diff [purge]

Index: trunk/phase3/languages/Language.php
——@@ -3820,7 +3820,7 @@
38213821 if ( $bps <= 0 ) {
38223822 return str_replace( '$1', $this->formatNum( $bps ), $this->getMessageFromDB( 'bitrate-bits' ) );
38233823 }
3824 - $unitIndex = (int)floor( log10( $bps ) / 3 );
 3824+ $unitIndex = (int)floor( log( $bps, 10 ) / 3 );
38253825 $mantissa = $bps / pow( 1000, $unitIndex );
38263826
38273827 $maxIndex = count( $units ) - 1;

Follow-up revisions

RevisionCommit summaryAuthorDate
r108288rv r108284 since PHP has different behaviors...hashar22:18, 6 January 2012

Comments

#Comment by Hashar (talk | contribs)   21:58, 6 January 2012

Of course PHP is great. On gallium: 

$ php -r 'print floor(log10(1000));'
3
$ php -r 'print floor(log(1000,10));'
2
$

My Mac gives 3 and 3: 

$ php -r 'print floor(log10(1000));'
3
$ php -r 'print floor(log(1000,10));'
3
$
#Comment by Hashar (talk | contribs)   22:01, 6 January 2012

I am probably going to drop log() entirely if it is not reliable.

#Comment by Reedy (talk | contribs)   22:04, 6 January 2012

Y U BREAK TESTS!?

#Comment by Hashar (talk | contribs)   22:19, 6 January 2012

Yeah the stupid PHP on gallium does not know how to log10( 1000 ) and pretends that is 2 :-(

PHP is broken!

I have reverted the change in r108288 to fix jenkins

#Comment by Platonides (talk | contribs)   23:27, 6 January 2012

What php version is used by Jenkins?

#Comment by Reedy (talk | contribs)   23:31, 6 January 2012 

reedy@gallium:~$ php -v
PHP 5.3.2-1ubuntu4.10 with Suhosin-Patch (cli) (built: Oct 14 2011 23:49:39)
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
#Comment by Hashar (talk | contribs)   09:40, 7 January 2012

See bug 33571

Status & tagging log