While doing some performance testing on some of our panels pages, I found that a one query in particular was executed 23 times per page. I believe the query is executed for each pane on the page (pane caching is not turned on). This query is generated from the function panels_load_displays() and the query looks something like this.

SELECT * FROM panels_pane WHERE did IN (1,2,3) ORDER BY did, panel, position;

I've created a patch to store each loaded pane into a static array, so that the query only has to run against IDs that have not been loaded yet. In our particular case the patch reduced the number of queries this function executes from 48 to 4.

CommentFileSizeAuthor
panels.load-displays-query.patch1.06 KBnrambeck

Comments

gerhard killesreiter’s picture

gerhard killesreiter commented
Status: Active » Needs review

This needs review

merlinofchaos’s picture

merlinofchaos commented

Interesting patch. Please note that one issue with this kind of caching is that it can drive up memory consumption. Is this an acceptable cost for this? Perhaps the loading is simply happening too often and could be trimmed at the source instead? I don't know the answer to that, but it should be explored.

sdboyer’s picture

sdboyer commented

I'm not too worried about the memory cost of a static cache, here - a highly complex Panel would maybe, _maybe_ end up reaching 75k of memory. Not a small amount individually, but the likelihood of that ever reaching more than 300k total (e.g., 4 highly complex panels) is quite remote. And paying 20-50k of memory for the more typical case in return for minimizing queries does seem like a generally worthwhile tradeoff.

What does worry me a bit is that in the quick benchmarking is that I was seeing more calls to this function than I expected on the page I was testing. Bears more investigation, because if there are unnecessary calls to this coming from somewhere, then it is possible we could end up with runaway static var growth.

Also...wow. I mean really, wow: 48 queries? Do you have a panel with 10 mini-panels on it, or something to that effect? This function isn't executed for each pane, it's executed once for each display, so I can't think of anything outside of a crap-ton of mini-panels that would result in that number of calls.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Needs work

Rereading this, this query is absolutely not intended to be executed for each pane on the page. I'm very curious how you're in an instance where that is happening. I've just doublechecked and in normal operations I'm not seeing many queries there at all.

I think this patch may actually be a bad idea; implementing it would only mask a deeper problem.

nrambeck’s picture

nrambeck commented

One of the reasons for the frequent call to this function originally was the ctools_content_get_subtype() function getting called frequently to get a list of blocks, including mini panels. That function now uses a static var cache in the latest version of ctools so it only calls the above function once per ctools content type.

Now, the panels_load_displays() function is called for the following reasons in my example panel page:

1. Load 1 display for the panel page
2. Load all mini-panel displays on the site. Source is the function ctools_content_get_subtypes($type="mini-panel') (this is called if there is 1 or more mini-panels in the page panel)
3. Load all mini-panel displays again. Source is the function ctools_content_get_subtypes($type="blocks') (this is called if any panel pane content is of a block type, even if you are not using a mini-panel block)
4. Load X number of mini-panels that exist in panel panes 1 by 1

Calls 1 and 2 are the only necessary ones, since by call #2 every possible display that could be displayed is already loaded.

If there is a deeper problem that can be fixed, all the better, but I'm not sure what that would be.

merlinofchaos’s picture

merlinofchaos commented

It sounds as though mini panel loads should very possibly be cached, then.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs work » Closed (duplicate)

And that has a patch already (that needs some work) http://drupal.org/node/609626