Suitecrm: Favorites getCurrentUserSidebarFavorites function is too costly

[jshwhitlow]

Issue

After analyzing my instance to improve performance I have found that a huge burden on resources is coming from the getCurrentUserSidebarFavorites function within modules/Favorites/Favorites.php

A breakdown of a 4.6 second load times revealed that 3.5 seconds of that time was spent on getCurrentUserSidebarFavorites and the time spent there was so large due to the person in question having 40 favorites.

Expected Behavior

This function needs to be optimized, retrieving a user with even just 10-15 favorites is very costly. Let alone 40+ favorites.

Actual Behavior

Issue already described above.

Possible Fix

Taking a look at the code in modules/Favorites/Favorites.php on line 103-141:

public function getCurrentUserSidebarFavorites($id = null)
    {
        global $current_user;
        $db = DBManagerFactory::getInstance();

        $return_array = array();

        if ($id) {
            $query = "SELECT parent_id, parent_type FROM favorites WHERE assigned_user_id = '" . $current_user->id . "' AND parent_id = '" . $id . "' AND deleted = 0 ORDER BY date_entered DESC";
        } else {
            $query = "SELECT parent_id, parent_type FROM favorites WHERE assigned_user_id = '" . $current_user->id . "' AND deleted = 0 ORDER BY date_entered DESC";
        }

        $result = $db->query($query);

        $i = 0;
        while ($row = $db->fetchByAssoc($result)) {
            $bean = BeanFactory::getBean($row['parent_type'], $row['parent_id']);
            if($bean) {
                $return_array[$i]['item_summary'] = $bean->name;
                $return_array[$i]['item_summary_short'] = to_html(getTrackerSubstring($bean->name));
                $return_array[$i]['id'] = $row['parent_id'];
                $return_array[$i]['module_name'] = $row['parent_type'];

                // Change since 7.7 side bar icons can exist in images/sidebar.
                $sidebarPath = 'themes/' . SugarThemeRegistry::current() . '/images/sidebar/modules/';
                if (file_exists($sidebarPath)) {
                    $return_array[$i]['image'] = SugarThemeRegistry::current()->getImage('sidebar/modules/' . $row['parent_type'], 'border="0" align="absmiddle"', null, null, '.gif', $bean->name);
                } else {
                    $return_array[$i]['image'] = SugarThemeRegistry::current()->getImage($row['parent_type'], 'border="0" align="absmiddle"', null, null, '.gif', $bean->name);
                }

                ++$i;
            }

        }

        return $return_array;
    }

We can see that its retrieving the bean for each and every favorite thats pulled back. This is way too costly considering that its only being used to pull back the bean->name.

Several possible solutions:

1. Create a new function that only retrieves the bean name to avoid the costly work of getting the entire bean just for one field.
2. Store the bean name in the database when the favorite is created and call it with sql instead of php, possible downside includes the bean name being outdated, so you'd have to check and update a favorite if one existed with a logic hook every time someone saved a record.
3. Introduce paging to the Favorites section, perhaps only 5-10 favorites at any one time, this could still be combined with suggestion 1) or 3) in order to reduce load times.

Steps to Reproduce

1. Add 10-20 favorites
2. Use some sort of analytics tool (I use New Relic, you can use other tools to track total time in php files)
3. Observe that most of the time spent loading is spent in BeanFactory::getBean being called over and over again by the getCurrentUserSidebarFavoritesfunction.

Context

We are trying to work on performance improvements, but are noticing that most of the custom code is not the issue, its the core SuiteCRM code that has a few areas for improvement. Being that this seems to be the core file that's obviously taking way way too long.

Your Environment

• SuiteCRM Version used: 7.10.6
• Browser name and version: Chrome Version 67.0.3396.99 (Official Build) (64-bit)
• Environment name and version (e.g. MySQL, PHP 7): MySQL 5.5.59, PHP 7.1.19
• Operating System and version: macOS 10.12.3

[horus68]

This is a most relevant question and a good way to approach issues in order to improve.
The code should be audited on "what functions are taking what amount of time" so it could then be a larger plan to reduce each function time.

[pgorod]

I would go with solution number 2, store the Bean name in the Favorites table.

For updates, we shouldn't put a logic hook (it's for every bean, every module), but we can simply update the generic Bean code that handles saves and _if (and only if) bean name was changed_, then run an update on the Favorites table also.

I'd say most records tend to have stable Names, so I don't expect the performance penalty to be too big.

[pgorod]

@jshwhitlow anyway, if you're getting screens taking many seconds to load, you might have additional problems in your database. You might want to investigate overgrown tables

https://pgorod.github.io/Database-tables-size/

Even if you do have specific problems and you can find a way to solve them, you are right that what you're reporting here should be efficient, it's coming up in every screen. So the issue is worth looking at, even if it's just to shave off 300ms or so.

[PedroErnst]

I would say exactly the same as @pgorod here: it might be that you have a different issue causing this particular slowdown.

I did some tests:

1. Empty dev DB, 0.07 s. to retrieve 20 favorites.
2. Production DB with 300k cases and 100k accounts, 0.18 s. to retrieve 20 favorites (10 cases and 10 accounts).

Both of these are on my local, so it's not even a proper server.

Now the initial idea is still valid I think, we don't need to retrieve all the beans to show the favorite bar. The getCurrentUserSidebarFavorites() function should probably take < 0.01 s. to execute.

[jshwhitlow]

@PedroErnst, @pgorod,

I'm definitely aware theres more than just this going on, we have to fix a lot of issues, one of which includes making sure the production database server and production code server are in the same city.

But I'm still digging into and trying to resolve some issues, I just had a 9 second load time today composed of:

• 4.5 seconds in SugarApplication::startSession
• 3.2 seconds in ListViewData::getListViewData
• 1.1 seconds in Favorites::getCurrentUserSidebarFavorites

I have no idea what the startSession is about and it only happens every so often. But the getListViewData seems to always take a long time. When I break down getListViewData, I see that:

SugarBean::ACLAccess was called 54 times within that function, and Opportunity::listviewACLHelper was called 20 times. The ACLAccess function clearly being the one consuming the most time at somewhere around 50ms on average, peaking at 109ms, but also as low as 4ms.

When I looked into ACLAccess on the 109ms time, I see that 99% of its time is spent in SugarBean::fill_in_relationship_fields. Within that function, BeanFactory::getBean was called 5 times, and the first 3 calls to it were 99% of that time.

If i follow the path down to which functions took the actual longest within getBean (these are not additive times, but rather, this function called another function and the ms time listed is how much time that function took inside the parent function):

BeanFactory::getBean (32ms) called SugarBean::retrieve (32ms) which called SugarBean::fill_in_relationship_fields (31ms) which called BeanFactory::getBean (20ms) which called Person::retrieve (20ms) which called SugarBean::retrieve (20ms) which called Lead::fill_in_additional_detail_fields (17ms) which called SugarBean::get_full_list (16ms) which called SugarBean::process_full_list_query (13ms) which called MysqliManager::query (12ms) which called "MySQL campaigns select" (12ms).

I find it interesting that on most of them the MYSQL select is usually what took the longest.

[pgorod]

Run the first query in this post

pgorod.github.io/Database-tables-size

I am almost sure you will find overgrown tables, namely securitygroups_records. You can purge this table of all orphaned records (pointing to records you no longer have in your DB)

[gunnicom]

Today i managed to find a performance issue a colleague had. It was the favorites.
Usually load times for me for a page is under 1 second. For him it was about 6-8 seconds. After deleting his favorites it came down to under 1 second again.
He had about 400 entries in the favorite table. I too often accidently click on the favorite star, and so does he (he never intended to add so many favorites).
So there is definitly a performance problem from the favorites. For now i will workaround it by deleting his favorites, but the function should be improved a lot. Maybe at least limit it to 10 or 20 favorites that get fetched.

[gunnicom]

So for me it was enough to add a "LIMIT 30" to the SQL in getCurrentUserSidebarFavorites.
Additionally it would be nice if the function wont run at all if you are using a theme like Suite7 because there is no Favorites-Sidebar. I am looking into this if it is possible to detect if a theme has a favorites sidebar or not.

[jshwhitlow]

@gunnicom, I don't think #6211 is a good solution, arbitrarily limiting the query to 30 does not actually solve the real issue. If the user has more than 30, they have no idea whats causing them to be limited to 30, nor they know how to get their favorites that are missing from the list.

Not to mention that we are still retrieving the bean for every favorite, which is whats really causing the slowdown.

Please reference one of the above solutions I have posted as a good workaround. Other users such as @pgorod concurred that storing the bean name in the favorites table would be the best solution, no need to limit to 30 at that point. If we want to do paging as well, then that needs to be incorporated so that the user still has access to all their favorites, not just the first 30.

[gunnicom]

@jshwhitlow I do not think this is an issue. I know this does not fix the deeper problem, but at least it fixes it without much hassle. Nobody wants to have 400 favorites in a list.
And if you limit there a maximum of 30 favorites will be retrieved, leading to a big performance win if someone has many favorites.
I do not think your argument that a user has no idea why it is limited to 30 is valid. The user has limits for entries in listviews, subpanels, menu tabs and so on. To address your issue maybe it can be configureable in config so the admin can tell the users why it is limited.

[jshwhitlow]

@gunnicom, well on my instance your solution will not help at all, my instance is still taking a long time due to the large quantity of fields for some beans being retrieved. Therefore, my instance will go un-helped to due to your "Hassle-Free" fix. 30 Favorites still takes several seconds on my instance.

There is no reason to slap a bandaid on this issue. The deeper problem is what this issue was opened specifically about. Sure, 400 favorites is excessive, and I do agree the system should have a limit of some kind, but we should solve the deeper, root issue now and then how many favorites there are is a moot point.

In the ListViews and subpanels there is a physical number showing the limited number of records to be pulled per page. The user knows how many records are being pulled back because it displays a count. This is not the case with the Favorites sidebar, no count is displayed, nor is there a way for the user to know which favorites are being excluded.

Example: If I have favorite 50 records, and have only 30 show, how as the user, do I know how to fix the issue? If remove one of my 30, one of the other 20 records will simply take its place.

[Mausino]

First we was happy that was created something like Favorites enhancement.. but by daily using of SuiteCRM shows us this problems

1. Favorites you can't add to listview (only detailview)
2. If you filter on listview of any module the Favorites.. it shows you all results... doesn't work it
3. favorites haven't own listview like Global search... separated by modules and on sidebar they are unusable... maybe you can add there 10 or more... but you lost in them
4. You can't create by default dashboard like My Favorites

The potential of this function is big... Now we don't use it anymore... the reasons see above

@Dillon-Brown should i create new ticket for this 4 points or let it be here ?

[gunnicom]

@jshwhitlow I totally agree with you that the deeper problem should be solved, but for now this little bandaid may help some users and is so easy to implement that it is a no brainer.
And to stay in the medical terms: I prefer getting a band aid and not bleed out than having to wait for maybe some month till i can see a doctor ;)

[jshwhitlow]

@gunnicom, @daniel-samson, I'm fine with the PR as long as this issue doesn't get closed and marked "Solved". The PR is a temporary fix, not the final fix that should be used to fully close out this issue.

[samus-aran]

Looks like this issue has raised alot of good suggestions. Lets break this down:

Performance Issue: @gunnicom has provided a quick fix that will help in the short term. This could grow into customising the side bar to be a configurable number...but we can't do that yet until we have a better solution that @jshwhitlow is asking for.
I agree with @PedroErnst & @pgorod that calling the bean for each record for a name is very draining and a solution would be to add name to the favourites vardefs and ensure that it saves the name when the favourite is saved. That sounds like the most immediate solution.

Enhancements: @Mausino has provided some very good suggestions (especially like the dedicated view for favourites) I wouldn't see this being too difficult to apply and we would definitely be interested in that if a community member creates a pull request. Additionally adding a dashlet probably won't be too difficult either. But I would suggest we get the performance issue resolve first properly as @jshwhitlow mentions :+1: