New subject: isActiveUser() and freeResult() both useless (was Re: [MediaWiki-CVS] SVN: [35867] trunk/phase3)

5 Jun 2008

brion(a)svn.wikimedia.org wrote:
...
  Revision: 35867
 Author:   brion
 Date:     2008-06-04 16:56:31 +0000 (Wed, 04 Jun 2008)

 Log Message:
 -----------
 Revert r35857, 35858, 35859 for the moment. Some notes:

 * Calling $user->isActiveUser() looks a bit redundant. :) Consider either shortening
it to isActive() or changing the last word to be clearer, say $user->isActiveEditor()
 * $wgActiveUserEditcount <- consider fully casing here; "editcount"
isn't a word. :D -> $wgActiveUserEditCount
 * $oldTime is dumped into SQL without any escaping or quoting. This will happen to work
on MySQL but will fail on PostgreSQL, and is generally a bad practice. Use addQuotes() to
ensure the formatted timestamp string is escaped and quoted for your raw SQL construct

There's also the slight problem that any attempt to use this 
User::isActiveUser() for the bugs referenced (13225 and 13585) would 
cause instant death of the site due to DB overload.

...
  * the database result object is not freed, which will
leak resources if used in a long-running script

It turns out that was just a misunderstanding on the part of the people 
who wrote the mysql extension for PHP. There is no need to free the 
underlying data for resource objects manually. The PHP manual is 
incorrect and mysql_free_result() is useless.

Underlying data for resources is deleted when the reference count for 
the resource goes to zero. This means assigning the variable to 
something else, or letting it go out of scope, works well enough.

You can easily test this by comparing the memory usage of this:

for ($i=0; $i<100000; $i++) { $res = $dbr->query('select * from revision 
limit 1000'); }

with this:

$res = array();
for ( $i=0; $i<100000; $i++) { $res[] = $dbr->query('select * from 
revision limit 1000'); }

The former uses virtually no memory, the latter uses lots.

-- Tim Starling