Cache the query in menu_local_tasks()

I just can't stomach using persistent caches as a mechanism to remove queries from page delivery. This is a single table, fully indexed query. It doesn't need any speeding up via cache.

I'm gonna leave this as needs review since I might be outnumbered here. Opinions welcome.

This is a no-brainer. Nothing can beat cache_get once it's out of MySQL. Free your mind. SQL is the slowest possible cache.

So, basically this cache entry shoudl only be invalidated at menu rebuild?

When else does it change?

It does not change otherwise as far as I know - so basically it's just when we clear this entire cache table?

Yes.

local_tasks.patch queued for re-testing.

Re-rolled against HEAD.

Needs to be fixed in d8 first

Looks like a bot issue.

#11: cache-menu-local-task-805236-11.patch queued for re-testing.

#11: cache-menu-local-task-805236-11.patch queued for re-testing.

local_tasks.patch queued for re-testing.

Converted the patch to Drupal 8 cache calls.

A cid is created but never used

$cid = 'local_tasks:' . $router_item['tab_root'];
if ($cached  = cache()->get('menu')) {

I assume it should be

$cid = 'local_tasks:' . $router_item['tab_root'];
if ($cached  = cache()->get($cid)) {

Reworked the arguments to the get and set and debugged to verify that the caching is occurring with repeated calls.
The comments in CacheBackendinterface.php still refer to the old cache_get() and cache_set().

Looking at the old patch, I think what this actually should do is cache('menu')->get($cid).

Updated with comment by Berdir. Ran all tests with and without the patch and compared the results. The only diff from the test outputs are:
< State system upgrade test 44 passes, 0 fails, and 0 exceptions
---
> State system upgrade test 42 passes, 0 fails, and 0 exceptions

Note that there are failures and exceptions with a complete test run without the patch (and these are identical with the patch);

Filter module filters 214 passes, 1 fail, and 0 exceptions
OpenID helper functions 43 passes, 2 fails, and 0 exceptions
Sort: Date 20 passes, 4 fails, and 0 exceptions

Entity Test Translation UI 68 passes, 0 fails, and 22 exceptions
Node translation UI 135 passes, 0 fails, and 18 exceptions
Taxonomy term translation UI 99 passes, 0 fails, and 22 exceptions

@merrillholt: There is no need to do complete manual test run, that's what the testbots are for. It is usually enough to just run the relevant/failing tests.

+++ b/core/includes/menu.incundefined
@@ -1894,13 +1894,20 @@ function menu_local_tasks($level = 0) {
+      cache('menu')->set($cid,$result);

Missing a space after the ,

added missing space.

#31: cache-menu-local-task-805236-15.patch queued for re-testing.

Rerolled to catch up with HEAD. Also reverted to comment # naming convention.

This will work differently in 8.x, just hasn't been completely removed yet, the LocalTask(Plugin?)Manager already has caching built in for what he does.

Moving back to 7.x, setting to needs work, an older patch probably still applies, but let's re-upload that one for clarity.

Thanks @Berdir! Tagging Novice to find the most recent patch on this issue that doesn't have 'core/' in the patch header (Dreditor will help!) and re-upload it.

Patch header that includes core/:

diff --git a/core/includes/menu.inc b/core/includes/menu.inc
index 47e1103..caacbdb 100644
--- a/core/includes/menu.inc
+++ b/core/includes/menu.inc
@@ -2052,13 +2052,20 @@ function menu_local_tasks($level = 0) {

#17, June 30, 2011- header is as follows, re-uploading now.

diff --git a/includes/menu.inc b/includes/menu.inc
index 3a376f2..4053e8e 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -1831,13 +1831,21 @@ function menu_local_tasks($level = 0)

Works fine. Thank you!

Like Moshe in #1, I'm pretty skeptical of this... but there haven't been any other complaints in the past 3 years, so I guess we should go with it.

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/aa6235c