Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow query with Structure when using channel:title parameter #2432

Closed
agun opened this issue Oct 14, 2022 · 5 comments · Fixed by #2554
Closed

Slow query with Structure when using channel:title parameter #2432

agun opened this issue Oct 14, 2022 · 5 comments · Fixed by #2554
Assignees

Comments

@agun
Copy link

agun commented Oct 14, 2022

Description of the problem
There is an issue with Structure when using the channel:title="channel:field" parameter. The issue is only seen when there are a lot of Structure entries (in my case 600+), and also lots of fields (100+ across the entire site). These entries are spread across a few channels. The issue is that there is 1 query that is taking almost 2 seconds to run every time the page is loaded (it doesn’t seem to get cached).

The query is in sql.structure.php on line 992 in the _get_page_titles() function:

// Get the Channel Entries that have are in a Structure Channel
$channelEntries = ee('Model')->get('ChannelEntry')
->filter('channel_id', 'IN', $structure_channel_ids)
->filter('site_id', '==', $this->site_id)
->all();

How To Reproduce
Steps to reproduce the behavior:

  1. You need to add in quite a lot of fields and entries into Structure (as pages)
  2. Use the channel:title="channel:field" parameter in a {exp:structure:nav_basic} tag
  3. Run the template ... and you will see it's very slow. I've isolated down the query issue to the one above.

Error Messages
There are no error messages. It works ... it's just VERY slow.

Screenshots / Videos / Template Code
N/A

Environment Details:

  • Version: EE7.1.6
  • PHP Version: 8.0.0
  • MySQL Version: 5.7.34
  • Web Server: Apache

Possible Solution
I've added some caching to the _get_page_titles() function, and this works really well after it's initially loaded (and the cache set). I think the Model could be re-written to be more performant with larger numbers of entries ... but I don't know how to do that.

@agun
Copy link
Author

agun commented Nov 15, 2022

I'm sure that there is a better way to do this, but it would be great to get some caching on the _get_page_titles() function in the sql.structure.php file. An example of the basic caching I've added is below. Currently for each upgrade that I do to the site, I'm having to re-add this into the core of EE (otherwise each page slows down by 2-3 seconds!).

private function _get_page_titles($sql_fields, $include_listings)
{
  $page_titles = ee()->cache->get('/structure/page_titles');

  if ($page_titles == '')
  {

    // we need the channel_id's of the structure entries
    $structure_channel_ids = $this->_get_structure_channel_ids($include_listings);

    // Get the Channel Entries that have are in a Structure Channel
    $channelEntries = ee('Model')->get('ChannelEntry')
        ->filter('channel_id', 'IN', $structure_channel_ids)
        ->filter('site_id', '==', $this->site_id)
        ->all();

    // Loop through the Channel Entries and create the page titles array
    $page_titles = array();
    foreach ($channelEntries as $channelEntry) {
        // Do not get the field, unless we prove that we should.
        $should_get_field = false;

        // This will determine if the custom title array actually has the data we need in it
        $custom_field_exists = (
            array_key_exists($channelEntry->channel_id, $sql_fields)    // the channel_id is in the array
                            && isset($sql_fields[$channelEntry->channel_id]['field_id'])    // The field_id is set
                            && !empty($sql_fields[$channelEntry->channel_id]['field_id'])   // The field_id is not empty
        );

        // if the custom field exists, lets check if the field actually has data in it in the channelEntry instance
        if ($custom_field_exists) {
            $field_id = 'field_id_' . $sql_fields[$channelEntry->channel_id]['field_id'];
        }

        // Set the page titles array accordingly
        if ($custom_field_exists && !empty($channelEntry->$field_id)) {
            $page_titles[$channelEntry->entry_id] = $channelEntry->$field_id;
        } else {
            $page_titles[$channelEntry->entry_id] = $channelEntry->title;
        }
    }

  ee()->cache->save('/structure/page_titles', $page_titles);

  }

  return $page_titles;

}

@intoeetive
Copy link
Contributor

I have investigated this and (while caching might actually solve this particular issue) found that this might be actually problem in EE core. Fetching ChannelEntry model would perform joins on all data tables for custom fields, even if you specify that you only need certain fields.

In some cases, like this one, this creates huge joins that take a while to execute.

We need to look into optimizing the load and not perform unnecessary SQL joins

@intoeetive intoeetive self-assigned this Nov 22, 2022
@agun
Copy link
Author

agun commented Nov 22, 2022

Thanks @intoeetive ... very much agree with you. For the moment, I'll need to leave the caching on this function (as it really is causing a very slow load, and it will only get worse as the site grows), but hopefully we can get this optimised at some point.

Thanks for looking into it.

@intoeetive
Copy link
Contributor

@agun can you try using the code from the pull request referenced here? I believe it should significantly speed up your query, but I don't have reliable way to test.

@agun
Copy link
Author

agun commented Nov 23, 2022

Hey @intoeetive ... I've now done some tests using the current core (no fix), my caching hack, and your new code (all tested on 7.2).

In a nutshell ... your fix works brilliantly! It's roughly as fast as my caching hack (maybe milliseconds slower, but not enough to worry about).

The current core (no fix) takes at least 2.5seconds LONGER to load than both the cache hack and your fix.

Thank you! I'm about to update to 7.2.1 ... but I'll roll your change into my version, and hopefully it'll be added to the core in the next few weeks!

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants