Would it be possible to cache the results of the _sections_in_section() function? On some of my pages this is getting 100s of times and querying the DB with the same query over and over again, accounting for a high percentage of my queries per page.

Could I fix this with some configuration change?

CommentFileSizeAuthor
#21 sections_repeated_queries-D6.patch3.21 KBhass
#12 sections_repeated_queries.patch1.61 KBrecidive
#10 sections-too-many-select-785554-10.patch4.07 KBsdelbosc
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

KingMoore’s picture

KingMoore CreditAttribution: KingMoore commented

I am not set up to roll a patch, but here is a modified version of the above function with very basic caching implemented by me:

/**
 * An API for modules that want to know about sections.
 *
 * This API is a function that lets you find out about settings.
 *
 * @param
 *  Optional $setting a string containing the section you what to test against.
 *
 * @return
 *   Depends on the parameter.
 *   If you do not give $section, it will return the section object, if found.
 *   If you give $section, it will return TRUE if you are in that section
 *   Otherwise it will return FALSE
 */
function _sections_in_section($section = NULL) {
  global $user;

  //my attempt at caching
  static $cached_row = array();
  
  if (is_string($section)) {
    // Caller wants to know if shes in the section she provided.
    if ($section == _sections_in_section()) {
      return TRUE;
    }
  }
  else {
    if(empty($cached_row)) {
      // Caller wants to know in which section she is.
      $rids = array_keys($user->roles);
      $res = db_query(db_rewrite_sql('SELECT DISTINCT s.* FROM {sections_data} s LEFT JOIN {sections_roles} r ON s.sid = r.sid WHERE s.status = 1 AND (r.rid IN ('. db_placeholders($rids) .') OR r.rid IS NULL) ORDER BY s.weight', 's', 'sid'), $rids);
      while ($row = db_fetch_object($res)) {
        if ($row->visibility < 2) {
          $path = drupal_get_path_alias($_GET['q']);
          // Compare with the internal and path alias (if any).
          $page_match = drupal_match_path($path, $row->path);
          if ($path != $_GET['q']) {
            $page_match = $page_match || drupal_match_path($_GET['q'], $row->path);
          }
          // When $row->visibility has a value of 0, the block is displayed on
          // all pages except those listed in $row->path. When set to 1, it
          // is displayed only on those pages listed in $row->path. Prevent
          // the admin theme switching on block admin pages.
          if ($page_match = !($row->visibility xor $page_match) && !(drupal_match_path($path, "admin/build/block\nadmin/build/block/list/*"))) {
            $cached_row = $row;
          }
        }
        else {
          if (drupal_eval($row->path)) {
            $cached_row = $row;
          }
        }
      }
    }

    return $cached_row;
  }

  // No section has been found, return FALSE.
  return FALSE;
}

On the specific page I am working on it reduces my number of queries by over 200.

hass’s picture

hass CreditAttribution: hass commented
Category: support » feature

Yes caching would be great and was planned!

hass’s picture

hass CreditAttribution: hass commented

Aside it's not normal that you have 100s... Schould be much less (2-3). Could you take a look where the 100s are comming from, please?

KingMoore’s picture

KingMoore CreditAttribution: KingMoore commented

It does this query:

 $res = db_query(db_rewrite_sql('SELECT DISTINCT s.* FROM {sections_data} s LEFT JOIN {sections_roles} r ON s.sid = r.sid WHERE s.status = 1 AND (r.rid IN ('. db_placeholders($rids) .') OR r.rid IS NULL) ORDER BY s.weight', 's', 'sid'), $rids);

repeatedly.

hass’s picture

hass CreditAttribution: hass commented

But - why - 100 times!?

KingMoore’s picture

KingMoore CreditAttribution: KingMoore commented

Yeah I have no idea. It was alarming to me. I have 4 sections defined like so:

1. Show on every page except the listed pages.
(none, so effectively the default)

2. Only on listed pages:
admin
admin/*

3. Only on the listed pages:
order-form

4. Only on the listed pages:
order-form-sell

I actually applied the code/patch I posted above in my working copy of sections module, so I can't recreate. It has worked for me so far without any negative effects that I can tell. Executing the following query only once:

SELECT DISTINCT s.* FROM drupal_sections_data s LEFT JOIN drupal_sections_roles r ON s.sid = r.sid WHERE s.status = 1 AND (r.rid IN (2,3) OR r.rid IS NULL) ORDER BY s.weight
hass’s picture

hass CreditAttribution: hass commented
Status: Active » Postponed (maintainer needs more info)

We need to understand first. It may be your default section that make no sense as your site defaults to a theme. You only need sections for other paths.

Please remove the patch and disable your first sectionand set your default theme to the section #1 theme. This is the recommended config.

Never create a section like your section #1

C-Logemann’s picture

C-Logemann
Primary language German
Location Frankfurt/M, Germany
CreditAttribution: C-Logemann commented

Hello Hass,
I currently working on a client project which was started by another company.
In this project we have up to 421 of these DB requests (_sections_in_section()) on <front> by using sections.modul.
There was no rule for default theme.
The snippet above don't change this situation.

On a small test system I have at least 26 DB requests on <front> by using sections.module.
With disabling flag.module I have only 17 requests. On the customer project flag is also active.
Perhaps in this way you will find a solution.

For my customer I have solved this problem by disabling sections and control the theme-switch in a custom module:
http://drupal.org/node/68932 and http://api.drupal.org/api/function/drupal_match_path/6

kind regards
Carsten Logemann
(paratio.com e.K.)

hass’s picture

hass CreditAttribution: hass commented

After more thinking I do have an idea... it's possible that Paratio have - let's say 10 node on the home and maybe a few boxes. This makes a sum of 17 reasonable... I'm aware about this, but haven't found the time to implement before the last release.

The changes from KingMore should work - without testing myself yet. Can you verify if you really have the DB requests with his changes or only the function calls?

sdelbosc’s picture

sdelbosc CreditAttribution: sdelbosc commented
FileSize
sections-too-many-select-785554-10.patch4.07 KB

I have a composite page with lots of menu, nodes, views, blocks, ... and I get more than 300 requests related to sections module per page.

I think it is due to hook_preprocess implementation which is probably called for many themable elements.
I do not understand the goal behind hook_preprocess implementation. But if it is not possible to do what is intended in another manner, I would suggest the patch attached to reduce the number of queries.

KingMoore’s picture

KingMoore CreditAttribution: KingMoore commented

Hass,

Sorry I have abandoned this thread. This is the really busy time of year for me. I will try to do/test what you asked in the next week sometime.

recidive’s picture

recidive CreditAttribution: recidive commented
Version: 6.x-1.4 » 6.x-1.x-dev
Status: Postponed (maintainer needs more info) » Needs review
FileSize
sections_repeated_queries.patch1.61 KB

Here is a simpler patch. It just adds _sections_roles() wrapper around the query and cache its results using role ids as key.

This patch solves the problem in a project I'm working on, without this patch the query runs 119 times, with the patch it runs only once.

sauloamui’s picture

sauloamui CreditAttribution: sauloamui commented

Work to me (#12)!
Really left only one query.

Thank´s recidive!

hass’s picture

hass CreditAttribution: hass commented
Status: Needs review » Needs work

Sorry, but this cannot be the right way... sections_data table is the basis... not roles. In may work in your special case, but if other modules call the sections module for different keys via _sections_in_section($section = NULL) we need to cache the results per $section.

recidive’s picture

recidive CreditAttribution: recidive commented

@hass, I may be missing something obvious. But what the patch does is just avoid this query to run a bunch of times when it can run only once per request.

That query only filters by role ids. So unless there's data going in and out of those tables in the same request, this query will always return the same result set for a given set of role ids.

I'm not aware of the inner workings of sections module. But, yes, caching per sections may be more efficient. And maybe this shouldn't even be a static cache but a database one.

This patch was an immediate solution for a project suffering from performance problems related to mysql overhead.

sdelbosc’s picture

sdelbosc CreditAttribution: sdelbosc commented

Hello hass. Could we have your opinion about the patch given in post #10 ?

hass’s picture

hass CreditAttribution: hass commented

Need to review and test it... sorry i was busy with google analytics.

KingMoore’s picture

KingMoore CreditAttribution: KingMoore commented

Just want to confirm that deleting my "default section" did not make any difference in the # of queries being performed.

hass’s picture

hass CreditAttribution: hass commented
Status: Needs work » Needs review

After more thinking and testing I came to the same result as recidive in #12. My first assumption that this is wrong - was incorrect. A user object could theoretically change at run time by other modules... so the rids must be the key. I'm doing some tests, but it looks very good.

hass’s picture

hass CreditAttribution: hass commented
Title: Statically cache sections data (Performance optimization) » caching of _sections_in_section ?
FileSize
sections_repeated_queries-D6.patch3.21 KB

Updated the patch from #12 with some documentation and changed naming. Commited code. THX.

Could the others in here please test and confirm, before we create a new release?

hass’s picture

hass CreditAttribution: hass commented
Title: caching of _sections_in_section ? » Statically cache sections data (Performance optimization)
Status: Needs review » Fixed

Title: caching of _sections_in_section ? » Statically cache sections data (Performance optimization)
Status: Fixed » Closed (fixed)

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

pixelpreview@gmail.com’s picture

pixelpreview@gmail.com CreditAttribution: pixelpreview@gmail.com commented

ok simply to say that the patch in #21 resolve the problem and remove a lot of "_sections_in_section()" in my site too