Jay Paroline – Grooveshark Dev

At Grooveshark we use DB-based php sessions so they can be accessed across multiple front-end nodes. As you would expect, the sessions table is very “hot,” as just about every request to do anything, ever, requires using a session. We noticed that web crawlers like google end up creating tens of thousands of sessions every day, because they of course do not carry cookies around with them.

The solution? Add a way to detect crawlers, and don’t give them sessions. Most of the solutions I’ve seen online look something like this:

function crawlerDetect($USER_AGENT)
{
$crawlers = array(
array('Google', 'Google'),
array('msnbot', 'MSN'),
array('Rambler', 'Rambler'),
array('Yahoo', 'Yahoo'),
array('AbachoBOT', 'AbachoBOT'),
array('accoona', 'Accoona'),
array('AcoiRobot', 'AcoiRobot'),
array('ASPSeek', 'ASPSeek'),
array('CrocCrawler', 'CrocCrawler'),
array('Dumbot', 'Dumbot'),
array('FAST-WebCrawler', 'FAST-WebCrawler'),
array('GeonaBot', 'GeonaBot'),
array('Gigabot', 'Gigabot'),
array('Lycos', 'Lycos spider'),
array('MSRBOT', 'MSRBOT'),
array('Scooter', 'Altavista robot'),
array('AltaVista', 'Altavista robot'),
array('IDBot', 'ID-Search Bot'),
array('eStyle', 'eStyle Bot'),
array('Scrubby', 'Scrubby robot')
);
foreach ($crawler as $c) {
if (stristr($USER_AGENT, $c[0])) {
return($c[1]);
}
}
return false;
}

Essentially, doing a for loop over the entire list of possible clients, and searching the user agent string for each one, one at a time. This seems way too slow and inefficient for something that is going to have to run on essentially every call on a high volume website, so I rewrote it to look like this:
public static function getIsCrawler($userAgent)
{
$crawlers = 'Google|msnbot|Rambler|Yahoo|AbachoBOT|accoona|' .
'AcioRobot|ASPSeek|CocoCrawler|Dumbot|FAST-WebCrawler|' .
'GeonaBot|Gigabot|Lycos|MSRBOT|Scooter|AltaVista|IDBot|eStyle|Scrubby';
$isCrawler = (preg_match("/$crawlers/", $userAgent) > 0);
return $isCrawler;
}

In my not-very-scientific testing, running on my local box my version takes 11 seconds to do 1 million comparisons, whereas looping through an array of crawlers to do 1 million comparisons takes 70 seconds. So there you have it, using a single regex for string matching rather than looping over an array can be 7 times faster. I suspect, but have not tested, that the performance gap gets bigger the more strings you are testing against.

One of my personal pet peeves is code that contains a bunch of conditional logic to handle seemingly special cases that really aren’t.

For example, let’s say we have a page that shows a user their playlists if they are logged in, but this page also has other useful information on it, and users aren’t required to log in. In our original design for our authentication class, if authentication “failed” (i.e. the user was not logged in), a call to Auth::getUserID() would return an empty string.

With that implementation, every piece that might have something to display to a logged in user has to have a bunch of conditional logic checking to see if the user is logged in, doing one thing i they are and another if they are not, adding unnecessary complexity and, of course, more potential for bugs to crop up.

This type of logic is completely unnecessary. I changed Auth to return a userID of 0 if the user is not logged in, and now it is not necessary to have any special handling for that case. If the user is logged in, they get playlists. If the user is not logged in, userID 0 does not have any playlists so they do not get playlists. I estimate that this simple change made over 100 lines of code obsolete. Not a whole lot of code in the grand scheme of things, but how many bugs can hide in 100 lines of code?

Another example of the same principle involves exceptions (which, as regular readers of Raymond Chen know, are hard to deal with). Under certain circumstances, recommendations from an external source can be included with our normal set of recommendations. The author of the recommendation-supplementing code originally had it throwing exceptions whenever it had problems. Of course, the case of not having supplemental recommendations because of an error is not really a special case. As far as my code is concerned, you just don’t have supplemental recommendations. In this case modifying the original code eliminates unnecessary handling for a special case and eliminates the potential for an uncaught exception to slip through.

A general rule of thumb to help avoid unnecessary special cases, at least with PHP, is to always return what you say you are going to return (and don’t throw exceptions). If your method processes some data which results in an array, return an array even if the processing has no results. This is the way most native methods in PHP already work, if you think about it. count(array()) doesn’t return an empty string or null or raise an exception, nor does count(null). Really, neither of these are special cases. In either case, the number of elements is zero, and developers are not required to care about the differences.