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

Remove multilanguage related db duplicate queries #8914

Conversation

andrepereiradasilva
Copy link
Contributor

Description

This PR, to remove duplicate queries from db, it saves into variables the result of the db queries of the two methods moved, only running the db query if needed.

Also moves some multilanguage methods from com_languages helper MultilangstatusHelper to JLanguage Multilang helper JLanguageMultilang.

Performance

This adds a little performance benefit. Around 2ms to 3ms of total page generation time (in a PHP 5.6 installation).

Before

image

After

image

How to test

  1. Apply patch in a default Joomla install.
  2. Check if a multilanguage site is working properly (frontend mod_languages works, frontend language filter works and admin multilanguage status works)
  3. Check if language associations are working properly
  4. Enable the debug console and see there are less queries (check before/after PR) and less ms taken to do all database queries.

Observations

Since this PR adds and deprecates some class methods, please say if anything else is needed to do.
Suggestions, code reviews and improvements are welcome.

@@ -78,41 +78,28 @@ public static function getContentlangs()
* Method to return a list of published site languages.
*
* @return array of language extension objects.
*
* @deprecated 3.6 Use JLanguageMultilang::getSiteLangs() instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deprecated 4.0

Always use the first version it may be removed from (so next major version), not the version it's deprecated in.

@infograf768
Copy link
Member

I get some Notices and Warning when the item has no associations yet.

( ! ) Notice: Undefined index: com_content|#__content|com_content.item|213|id|alias|catid in /ROOT/libraries/cms/language/associations.php on line 121

or
( ! ) Notice: Undefined index: com_menus|#__menu|com_menus.item|631|id|| in ROOT/libraries/cms/language/associations.php on line 121

with for example
( ! ) Warning: Invalid argument supplied for foreach() in ROOT/trunkgitnew/administrator/components/com_menus/helpers/menus.php on line 321

I can get rid of the Warning for example in the menus helper by checking if we do have associations:

    public static function getAssociations($pk)
    {
        $langAssociations = JLanguageAssociations::getAssociations('com_menus', '#__menu', 'com_menus.item', $pk, 'id', '', '');
        $associations = array();

        if ($langAssociations)
        {
            foreach ($langAssociations as $langAssociation)
            {
                $associations[$langAssociation->language] = $langAssociation->id;
            }
        }

        return $associations;
    }

But I still have the Notice.
I could get rid of it by adding a conditional, but not sure it is the best way.

        if (!empty($multilanguageAssociations[$queryKey]))
        {
            return $multilanguageAssociations[$queryKey];
        }

@andrepereiradasilva
Copy link
Contributor Author

sorry @infograf768 code was a problem, i willl fix it soon

@andrepereiradasilva
Copy link
Contributor Author

i think it's solved now, the bug was is line 47 andrepereiradasilva@167035f#diff-5ca4c28b8e63fb73e99b5128c808630fR47 (the variable name was wrong)

…e-language-duplicate-queries

solves conflict
@infograf768
Copy link
Member

I have tested this item ✅ successfully on b9d67c2

Wow! This works real good here. Thanks for this PR.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8914.

@MATsxm
Copy link

MATsxm commented Jan 20, 2016

I have tested this item ✅ successfully on b9d67c2

No problems here!
Thanks 👍


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8914.

@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on b9d67c2


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8914.

@dgrammatiko
Copy link
Contributor

RTC then!

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 20, 2016
@wilsonge wilsonge closed this in c684036 Jan 21, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 21, 2016
@andrepereiradasilva andrepereiradasilva deleted the remove-language-duplicate-queries branch January 22, 2016 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants