Home » File Formats » Spreadsheet_Excel_Writer » Bug #5313

[2005-09-06 19:09 UTC] jamie dot alessio at ucop dot edu

Description:
------------
In the process of creating an excel file with eleven worksheets I ran into performance problems. The script was taking over 30 seconds to generate an 80Kb file. I used xdebug2 (www.xdebug.org) to get profiling information on the script and discovered that the majority of the execution time was spent in the Spreadsheet_Excel_Writer code.

The biggest performance hit was from calls to the PHP strlen() function. The strlen() function was being called 116,930 times and was consuming 58% of the time of the entire script. I tracked down the problem to the Spreadsheet_Excel_Writer_Parser::_advance() function and noticed that strlen() was being called several times as part of a while() loop. Instead of calling the strlen() function several times in a loop, I rewrote the function to store the return value of strlen() at the beginning of the loop and then used the variable instead of calling strlen() so many times.

Example....

///////////////////////////////////////////////
// Original code
while ($i < strlen($this->_formula))
{
// do something here
}

// New code
$formula_length = strlen($this->_formula);
while ($i < $formula_length)
{
// do something here
}
//
///////////////////////////////////////////////

This simple change reduced the number of calls to strlen() to 43,119 (down from 116,930) and reduced the script execution time by over 15 seconds!

I can provide a full diff for changes, but it is a simple fix and will become apparent as soon as you look at the Spreadsheet_Excel_Writer_Parser::_advance() function.

The next big performance hit is from calls to is_array() in the recursive function Spreadsheet_Excel_Writer_Parser::toReversePolish() but a quick fix for that problem isn't as readily apparent. For reference, in my script is_array() is being called 42,840 times from toReversePolish() and is consuming 35% of the execution time.

Let me know if you'd like test cases or a diff for this, but I think you'll see what I'm talking about once you take a look at Spreadsheet_Excel_Writer_Parser::_advance().

This is very similar to this fixed bug in PEAR::DB
http://bugs.php.net/bug.php?id=23698

[2005-09-06 19:14 UTC] jamie dot alessio at ucop dot edu

just fixing typo in 'Summary'

[2005-10-24 18:00 UTC] jamie dot alessio at ucop dot edu

I profiled the new code you put in the CVS repository with xdebug. Your updates provided performance gains similar to those I described after modifying the code myself - it significantly reduced the number of calls to strlen() (116,930 calls down to 34,421 in my test) which cut the script execution time roughly in half. This looks like a no-brainer to add to the stable release of the package.

>> Regarding the is_array issue, could you try
>> doing a isset() before the is_array and tell
>> me if that helps anything ?
>>
>> i.e. isset($foo) && is_array($foo) that's known
>> to sometimes speed things up, just want a
>> confirmation in this case.
>>
I added the isset() calls before the is_array() calls and profiled that code too. The addition of isset() did not reduce the number of calls to is_array() and did not provide any speed improvement. is_array() is only called in two places in the Spreadsheet_Excel_Writer_Parser::toReversePolish() function and in both cases I believe the variable will *always* be set so that the addition of the isset() call doesn't provide any benefit. Again, without really digging into the code here I don't see any easy wins on this one.

I'll leave this bug open for now but as far as I'm concerned the main problem that I opened the bug for has been addressed. Thanks for taking the time to update the PEAR code.

[2005-11-12 03:50 UTC] xnoguer at php dot net

This bug has been fixed in CVS.

If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET).

If this was a problem with the pear.php.net website, the change should be live shortly.

Otherwise, the fix will appear in the package's next release.

Thank you for the report and for helping us make PEAR better.

I'm closing this one now. I'll release a new version soon, including the improvements made by dufuz's.