Dashboard, region and block building have unnecessary function calls

Confirming it's slightly faster with the patch... but just slightly.

ab -c1 -n100 -C [session cookie] http://localhost/admin/dashboard
unpatched: 6.69 req/sec
patched: 6.76 req/sec

Could you elaborate on the problem with block building and rendering? We don't usually have many blocks on a page so I would surprised if thats a significant contributor.

I think when you look at a call graph, block rendering includes the rendering of the main content block, so that's probably what's showing up, but haven't looked into it much.

We are currently attempting to render the dashboard on every single page though, which is stupid.

@sun - can you re-roll without the call_user_func_array() changes?

We are currently attempting to render the dashboard on every single page though, which is stupid.

As far as I recall, we specifically make sure not to render it - see http://api.drupal.org/api/function/dashboard_block_info_alter/7

So I'm not sure I understand how this issue is about anything more than micro-optimization? (Not to say that it isn't worth doing, of course, but I don't understand how this would lead to major performance improvements.)

Here's what shows up when viewing the front page.

Looks about right to me.

What is the rationale for using static rather than drupal_static() here?

Much more accurate title and priority, IMO :)

Committed to CVS HEAD. Thanks!

Well, I thought the default policy was to use drupal_static() unless there is a really good reason not to... but the reality is that there is no policy :)

In any case, I think we do need it for dashboard_regions() since the results of that function come from module_invoke_all(), so it seems like they could change. Probably don't need it for dashboard_is_visible().

I thought the policy is to use drupal_static only where you need to be able to reset the cache in special situations (those situations where a $reset parameter was formerly used). If you wouldn't have used a $reset parameter before, you wouldn't use a drupal_static now. So as sun noted, the question should always be: What's the rationale for using drupal_static? instead of the other way around.
About the module_invoke_all(). Can't this only change if a new module is activated? In that case it would be before dashboard_regions() is called for the first time, or not?

Well, module_enable() can be called at any time in a page request or while running a script, and we can't predict when that will be in relation to anything else...

No longer a performance issue.

The results of module_enable() definitely work correctly within the same page request. If they didn't, installing Drupal via the command line would not work..... And although it's true that you can't see the dashboard in a command line script, you still might want to query information about it.

In any case, there is now (voluminous) documentation at http://api.drupal.org/api/function/drupal_static/7 about when drupal_static() should and shouldn't be used, and although I think the whole drupal_static() thing is a bit weird to begin with, IMO that clearly indicates it should be used here. It's also a lot better than trying to document why it shouldn't be used, which is now done in a lot of places in core that intentionally skip use of drupal_static :)

Patch in #14 still applies....

dashboard_regions()'s static variable does depend on information outside of the function and that might change during a single page request.

Committed to HEAD. Thanks!

Back to original title and priority, for posterity.