Cache query in context_reaction_block::get_blocks() & remove unused blocks before the alters get called.

If using panels_mini, this can save around 10ms per block removed. Also caching the query helps by another 20ms.

Patch also fixes some minor white space issues.

Any movement on this patch? We've been using it successfully on a site for a while.

I'd mark as RTBC, but would be good to have others give their feedback.

Well you know I've been using it as well. There's not a lot of followers on this issue so go ahead and make it RTBC.

Done, thanks...

Can you add tests for this?

Aside, why is the result cached unaltered? I'm not sure if this is best practice...

why is the result cached unaltered

My reasoning was that the alter might do something based off of some external context (like user, url, theme, etc). Currently doing a search in Gotta Download Them All via grep -rn -e "_context_block_info_alter(" ./ to see if I can find any use cases where this is true.

Looking over it and the hook gets implemented in 3 places; most complex is inside of the i18n_block module.
defaultcontent.module - defaultcontent_context_block_info_alter(&$blocks)
i18n_block.module - i18n_block_context_block_info_alter(&$block_info)
blocks_mass_cache.module - blocks_mass_cache_context_block_info_alter(&$blocks)

I'm not 100% sure caching the alter would be safe due the the usage of theme_default; see http://drupal.stackexchange.com/questions/161300/page-caching-wrong-css for an example of this being dynamic.

Opps wrong hook
http://cgit.drupalcode.org/context/tree/plugins/context_reaction_block.i...

Looking at these 2

        drupal_alter('block_list', $result);
        drupal_alter('context_block_list', $result);

Nothing implements hook_context_block_list_alter but there's a lot of modules that implement hook_block_list_alter. This alter can not be cached.

Also noted that the theme is appart of the cid for this cache so the previous alter would have been ok; issue is hook_block_list_alter is used everywhere, pulling in external context that we can never know about, thus making it uncacheable.

So a test case would be to make sure that hook_context_block_list_alter only gets blocks for the given region and not all of the blocks. Is there any coverage for get_blocks currently?

No idea. I'm new to this module and just fixed a bunch of menu issues. I'd only like to prevent failures in future.

+ RTBC makes a big difference for us.

• Needs tests.
• This patch introduces a new cache key but does not flush that cache. Presumably the cache needs to be flushed when block data change.

Setting to needs work.