This does a db_query()/module_invoke_all() as many times as it's requested, could use a static cache.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch’s picture

Status: Active » Needs review

Main reason to static cache this is that any menu link to a homebox is going to result in this being called.

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

I tested this on simplytest.me. Works as expected.

I didn't do any performance testing, but drupal_static() makes sense to me.

maximpodorov’s picture

Status: Reviewed & tested by the community » Needs work

The cache must be cleared after saving homebox page in homebox_save_page().

mgifford’s picture

Status: Needs work » Needs review
Issue tags: +drupal.org performance
FileSize
1.58 KB

So like this. Good catch btw.

mgifford’s picture

Sorry, there was some cruft in that last patch.

maximpodorov’s picture

Status: Needs review » Reviewed & tested by the community

Looks good and works.

drumm’s picture

Status: Reviewed & tested by the community » Needs work

homebox_delete_page() also needs to clear the cache for the deleted page.

In homebox_get_page():

    $names[$name] = $page;

    return is_object($page) ? $page : FALSE;

Setting the cache should also check is_object($page).

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.85 KB

Thanks. I believe this addresses your two points.

  • drumm committed 1a12d8f on 7.x-2.x authored by mgifford
    Issue #2153629 by mgifford, catch: homebox_get_page() should static...
drumm’s picture

Status: Needs review » Fixed

Looks good, committed with a little cleanup.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.