theme_token_tree uses lots of memory

I just posted #1068990: Optimize and add caching for theme_token_tree() and saw this issue.

Since I had things open, I profiled admin/modules with token enabled, and it's responsible for up to 10mb of memory usage on that page. See attached screenshot. There is only one call to theme_token_tree() on that patch, so the static caching in the other issue won't help.

Re-classifying to major.

The reason this is called at all on admin/modules looks to be due to token_help().

Seems like this should be a duplicate of that issue, then?

Or do you think we can do more to improve performance of this function even for cases where it's only called once?

I'd like to leave this open to see if we can improve the overall performance of the function (and/or clean up memory) - the caching in the other issue isn't going to help admin/modules and that's already a memory intensive page (although there might be a workaround in hook_help() to stop the function being called from there).

Dave said in the other issue:

Should I also consider calling theme_tree_table() directly rather than invoking a theme function from another theme function?

This is worth comparing, I should be able to profile that tomorrow.

Started looking at this.

Used more or less clean install + token, and used admin/help/token for the profiling since that's rendering all the available tokens in a single tree. The number of tokens is going to be higher on some installs but core tokens plus the ones token provides is still plenty.

Began by looking at _token_token_tree_format_row() since that's called 1,365 times on this page and took 40ms out of 168ms for the whole thing.

Made two changes there:

1. Inlined _token_clean_css_identifier() into a direct strtr() call - similar logic to #680022: template_preprocess() is too slow in generating valid CSS class name.
2. Added a static cache for the cleaned version of token parents, this reduced the calls to strtr() by about 45% (from 2715 to 1550).

This brought it down from around 40ms to around 18ms.

Peak and incl. memory usage only came down by 200k (out of nearly 5mb), will need to try to figure out why that's so bad.

Then I had a look at theme_tree_table(). This was calling drupal_clean_css_identifier($row['parent']); But $row['parent'] has already gone through token_clean_css_identifier() (or strtr() now). Cutting that out saved another 1350 calls to it, 253k of memory (800k peak) and about 10ms.

Added a single count for multiple token types at the top of the theme_token_tree(), this removes 1,365 calls to count(), that's minimal for wall time but apparently 7% of the cpu usage of theme_token_tree().

At this point xhprof summary looks like this, note the time values here aren't very reliable since it's a single run, memory and function calls are usually accurate though unless something misses a cache etc.

HEAD:

Total Incl. Wall Time (microsec):	327,669 microsecs
Total Incl. CPU (microsecs):	310,000 microsecs
Total Incl. MemUse (bytes):	31,117,712 bytes
Total Incl. PeakMemUse (bytes):	33,487,520 bytes
Number of Function Calls:	65,797

Patch:

Total Incl. Wall Time (microsec):	290,800 microsecs
Total Incl. CPU (microsecs):	280,000 microsecs
Total Incl. MemUse (bytes):	31,194,520 bytes
Total Incl. PeakMemUse (bytes):	33,434,752 bytes
Number of Function Calls:	49,908

Going to keep looking at this but posting for now since I have a feeling the changes are going to get more hacky from here on..

I was able to shave 10% off the cost of strtr() (and 40% of the cost of trim()) by keeping the known 'token-' out of the replacement text and just concatenating it at the beginning.

Looked at preg_replace() (with two arrays) vs. strstr and preg_replace() came out both slower and more memory. Maybe preg_replace() with a proper regexp is worth comparing but I suck at regular expressions.

If some of the replacements could be skipped too that might help, but I don't know either way.

For memory usage, nearly 5mb of memory is spent in _token_token_tree_format_row(). I was able to save 700k from this, by adding a static containing the default elements of the array, then adding the values selectively, instead of building the array from scratch each time. I don't really understand why this works but it seems like copy on write + reference counting helps even if you modify some parts of the array.

This only equates to about 300k of memory savings over the entire page though, so not that great but something.

_token_tree_format_row() now looks like this:

HEAD:
Function Name	Calls	Calls%	Incl. Wall Time (microsec)	IWall%	Incl. CPU (microsecs)	ICpu%	Incl. MemUse (bytes) IMemUse%	Incl. PeakMemUse (bytes)	IPeakMemUse
HEAD:
_token_token_tree_format_row	1,365	49.3%	39,023	23.2%	40,000	25.0%	4,956,096	138.8%	4,335,688
Patch:	
_token_token_tree_format_row	1,365	97.0%	17,237	11.6%	0	0.0%	4,283,008	117.6%	3,656,904	35.8%

I think this is about as far as I can get with it, about a 15% improvement, not bad (and 82ms out of (currently) 128ms is spent in theme_table() which we can't control so much) but I'm really thinking about persistent caching of this in the other issue now.

Some of the stranger memory numbers are starting to make sense. As far as I can tell xhprof inclusive peak memory usage means the total memory usage of each function call within the function being examined - whether or not the memory used from those functions is freed up after they're called or not.

So _token_tree_format_row() incl. peak memory usage might use 2,648,120 bytes of memory, but if it's called 1,365 times then that's 1,940 bytes being used each time, and some percentage of that is freed at the end of the function call. So it's interesting but it's not really correlated to the total memory use during the request at any one time.

I ran the following two functions 100,000 times each and profiled in xhprof to see the difference.

It looks like having the static, copying it, then modifying the version and returning it is about four times as efficient as just building the array each time.

Also tidied up how the $defaults array is declared, the !isset etc. is pointless.

I'm going to close this as a duplicate of #1068990: Optimize and add caching for theme_token_tree() since the patches conflict, re-open if you want to do these one at a time.