? Success
Pull Request for # 9516

User tests: Successful: Unsuccessful:

avatar alikon
alikon
31 Mar 2016

Pull Request for Issue #9516 .

Summary of Changes

query clean (distinct instead of group by)

Testing Instructions

edit a module item from backend with debug plugin enabled and look for
before0
apply the patch and edit a module
after0

avatar alikon alikon - open - 31 Mar 2016
avatar alikon alikon - change - 31 Mar 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Mar 2016
Labels Added: ?
avatar JoshuaLewis JoshuaLewis - test_item - 31 Mar 2016 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 31 Mar 2016

I have tested this item :white_check_mark: successfully on 77abb6d


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

avatar brianteeman brianteeman - change - 31 Mar 2016
Category SQL
avatar brianteeman brianteeman - change - 31 Mar 2016
Rel_Number 0 9516
Relation Type Pull Request for
avatar brianteeman
brianteeman - comment - 31 Mar 2016

Thanks I will test in the morning with the large dataset


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

avatar brianteeman brianteeman - test_item - 1 Apr 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 1 Apr 2016

I have tested this item :white_check_mark: successfully on 77abb6d

Tested with 4000 menu items

Before

Time: 8506.36 ms / 8620.83 ms Memory: 8.762 MB / 13.74 MB Application: afterRenderComponent com_modules

After

Time: 673.98 ms / 780.86 ms Memory: 8.760 MB / 13.75 MB Application: afterRenderComponent com_modules

WOW!!!!!


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

avatar brianteeman brianteeman - change - 1 Apr 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 1 Apr 2016

RTC


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

avatar joomla-cms-bot joomla-cms-bot - change - 1 Apr 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 1 Apr 2016
Milestone Added:
avatar benshaty
benshaty - comment - 3 Apr 2016

it still slow but if i cancel the multilang query it work good in the template manager.

before change: 10000ms
image

after change: 93ms
image

line 165:
if (JLanguageMultilang::isEnabled())
{
$query->select('l.title AS language_title, l.image AS language_image')
->join('LEFT', $db->quoteName('#__languages') . ' AS l ON l.lang_code = a.language');
}

is the lang query necessary?

i found another problem that in the module manager in menu assignment if i choose
Only on the pages selected / On all pages except those selected
the view of the menus are defective:

image

thanks in advance

avatar ggppdk
ggppdk - comment - 3 Apr 2016

About the broken tree display
Do you imply that this is related to this PR ?

  • it looks like a language related

so maybe posting this here is not relevant ?

Does it occur with the original SQL query (without changes of this pull request) ?
does it occur if you turn off language debug ?
does it occur when you removed the join clause with the language table ?

avatar benshaty
benshaty - comment - 3 Apr 2016

Do you imply that this is related to this PR ? - yes i did (i reported the bug)
Does it occur with the original SQL query (without changes of this pull request) ? - now i see that it occur even if i put the original file (the before PR file) so maybe it`s another bug
does it occur if you turn off language debug ? - yes
does it occur when you removed the join clause with the language table ? - yes

avatar ggppdk
ggppdk - comment - 3 Apr 2016

@benshaty
About the language join the data of it are need and used later:
why your SQL server cannot optimize this (we will need to look at explain)

@benshaty , @alikon , @brianteeman
please test the following:

Put it in comments (the language JOIN)

/*if (JLanguageMultilang::isEnabled())
{
    $query->select('l.title AS language_title, l.image AS language_image')
        ->join('LEFT', $db->quoteName('#__languages') . ' AS l ON l.lang_code = a.language');
}*/

and replace it with an extra query on the languages table that

  • will only retrieve a handful of language records: (the SQL cost of the new query is almost zero, and PHP cost of it is 2 PHP loops with 4000 iterations each, which in anyway is also minimal, (the created language array is also a few rows))

Find lines:

try
{
    $links = $db->loadObjectList();
}

Replace them with:

try
{
    $links = $db->loadObjectList();
    if (JLanguageMultilang::isEnabled())
    {
        // 1. Find codes of needed languages
        $lcodes = array();
        foreach($links as $link) $lcodes[$link->language] = 1;

        // 2. Query the languages table
        $lcodes_escaped = array();
        foreach($lcodes as $code => $i) $lcodes_escaped[] = $db->quote($code);

        $query = $db->getQuery(true)
            ->select('title, image, lang_code')->from( $db->quoteName('#__languages') )
            ->where('lang_code IN ('.implode(',', $lcodes_escaped).')');
        $db->setQuery($query);

        $langs = count($lcodes) ? $db->loadObjectList('lang_code') : array();

        // 3. Set needed language data into the menu links, (existing template code will work)
        foreach($links as $link)
        {
            $lang = @ $langs[$link->language];
            $link->language_title = $lang ? $lang->title : '';
            $link->language_image = $lang ? $lang->image : '';
        }
    }
}
avatar alikon
alikon - comment - 3 Apr 2016

@benshaty "template manager" ???? can you put a screenshoot ?
quite strange thing from your prev screenshoot 3 duplicate queries
do you have any 3dp extensions/template ?

avatar ggppdk
ggppdk - comment - 3 Apr 2016

Instead of getting 2-10 rows from the languages table,

  • we duplicate language title and language image on every row that the (menu items) query creates, and then we ask SQL server to sort and filter (DISTINCT) much larger records, that need more memory to be accommodated

and how many rows are created before sorting and filtering occurs ?

  • because of the sub-tree join on lft, rgt that would be a lot ? proportional to nn ? aka O(nn) ? , and his web-site has 3000 + menu items, maybe after removing the language join the query data fits inside CPU cache ?

Anyway the join with the language tables does so much unneeded data replication

  • that is better to remove it [EDIT] i meant replace it
avatar benshaty
benshaty - comment - 3 Apr 2016

@alikon this is the screenshot of the template menu assignment:
image

my template is t-3_bs_blank

@ggppdk didn`t work on module manager:
image

now when i try to close module / t-3 template it stuck for a few seconds but work after few seconds.
all the data:
image

avatar alikon
alikon - comment - 3 Apr 2016

so is not a core issue as you are using a non core template

avatar benshaty
benshaty - comment - 3 Apr 2016

@alikon the problem is not in the template but the module manager

avatar ggppdk
ggppdk - comment - 3 Apr 2016

@ggppdk didn`t work on module manager:

why would did it ever fix the broken display ? ))
did i say it would do anything about the broken display ?

  • i meant to test it, in order to fix the performance with the language table join

Now about the delay on form save,

  • it is expected considering the current validation JS code
  • it is a different issue and there is a pending pull request that excludes menu items from the validation JS (if i remember correctly)
avatar alikon
alikon - comment - 3 Apr 2016

let me disagree , is your template code that execute the query 3 times

avatar benshaty
benshaty - comment - 3 Apr 2016

@alikon now the template is working good but the order of the modules are defective

avatar rdeutz rdeutz - change - 12 Apr 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-04-12 20:17:54
Closed_By rdeutz
avatar rdeutz rdeutz - close - 12 Apr 2016
avatar rdeutz rdeutz - merge - 12 Apr 2016
avatar joomla-cms-bot joomla-cms-bot - close - 12 Apr 2016
avatar rdeutz rdeutz - reference | f37c332 - 12 Apr 16
avatar rdeutz rdeutz - merge - 12 Apr 2016
avatar rdeutz rdeutz - close - 12 Apr 2016
avatar joomla-cms-bot joomla-cms-bot - change - 12 Apr 2016
Labels Removed: ?
avatar alikon alikon - head_ref_deleted - 13 Apr 2016
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Removed:
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Added:
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Added:
avatar rdeutz rdeutz - change - 1 May 2016
Milestone Removed:

Add a Comment

Login with GitHub to post a comment