Moodle
  1. Moodle
  2. MDL-29831

Multiple occurrences of the editor on the same page cause excessive db queries

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      Enable perf to footer for ease
      From Home as administrator:

      • Settings -> Front Page Settings -> Questions
      • Select 'Create a new Question'
      • Choose 'Multiple Choice'
      • Note number of queries
      • Refresh page
      • Note number of queries
      • From the breadcrumbs, choose 'Edit question'
      • Select 'Create a new Question'
      • Choose 'True/False'
      • Note number of queries
      • Refresh page
      • Note number of queries

      Enable an additional Repository plugin (I chose URL Downloader with default settings)
      Repeat

      Show
      Enable perf to footer for ease From Home as administrator: Settings -> Front Page Settings -> Questions Select 'Create a new Question' Choose 'Multiple Choice' Note number of queries Refresh page Note number of queries From the breadcrumbs, choose 'Edit question' Select 'Create a new Question' Choose 'True/False' Note number of queries Refresh page Note number of queries Enable an additional Repository plugin (I chose URL Downloader with default settings) Repeat
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29831-master
    • Rank:
      19356

      Description

      On a page with multiple copies of the editor (e.g. when adding a new multichoice question), an excessive number of database reads are seen.

      When the editor is inserted into a page, lib/form/editor.php::MoodleQuickForm_editor->toHtml() is called
      This in turn calls initialise_filepicker once for each type of media plugin - e.g. advimage, moodlemedia, and advlink
      The initialise_filepicker() function in turn calls repository::get_instances which generates a set of queries for each of the repository plugins.

      As an example on my default 2.1 install:

      First editor: 57 queries
      Subsequent editors: 54 queries each
      Total queries for 17 editors = 57 + (54 * 16) = 921
      

      Enabling an additional repository plugin (URL downloader) changes these numbers

      First editor: 69 queries
      Subsequent editors: 66 queries each
      Total queries for 17 editors = 69 + (66 * 16) = 1125
      

      (Note, I loaded the page twice after adding enabling additional repository plugins)

      We should see if it's possible to cache the results for some of these queries. They're mostly of the nature:

      SELECT * FROM mdl_repository_instances WHERE id = $1 [array ( 0 => '1', )]
      SELECT * FROM mdl_repository_instance_config WHERE instanceid = $1 [array ( 0 => '1', )]
      SELECT * FROM mdl_repository WHERE id = $1 [array ( 0 => '1', )]
      SELECT name,value FROM mdl_config_plugins WHERE plugin = $1 [array ( 0 => 'local', )]
      

      This is seen for any user (not just administrators). The question component is not at fault, but provides a handy set of editors to try.

        Issue Links

          Activity

          Andrew Nicols created issue -
          Andrew Nicols made changes -
          Field Original Value New Value
          Description On a page with multiple copies of the editor (e.g. when adding a new multichoice question), an excessive number of database reads are seen.

          When the editor is inserted into a page, lib/form/editor.php::MoodleQuickForm_editor->toHtml() is called
          This in turn calls initialise_filepicker once for each type of media plugin - e.g. advimage, moodlemedia, and advlink
          The initialise_filepicker() function in turn calls repository::get_instances which generates a set of queries for each of the repository plugins.

          As an example:
          The first time the editor is displayed, on my default 2.1 install I see 57 queries
          For each subsequent time the editor is displayed, I see a further 54 queries.
          On the multichoice question type, there are 17 editors, so 57 + (54 * 16) = 921 queries

          Enabling an additional repository plugin (URL downloader) changes these numbers
          The first time the editor is displayed: 69 queries
          For each subsequent time the editor is displayed, I see a further 66 queries.
          On the multichoice question type, there are 17 editors, so 69 + (66 * 16) = 1125 queries

          (Note, I loaded the page twice after adding enabling additional repository plugins)

          We should see if it's possible to cache the results for some of these queries. They're mostly of the nature:
          SELECT * FROM mdl_repository_instances WHERE id = $1 [array ( 0 => '1', )]
          SELECT * FROM mdl_repository_instance_config WHERE instanceid = $1 [array ( 0 => '1', )]
          SELECT * FROM mdl_repository WHERE id = $1 [array ( 0 => '1', )]
          SELECT name,value FROM mdl_config_plugins WHERE plugin = $1 [array ( 0 => 'local', )]

          This is seen for any user (not just administrators). The question component is not at fault, but provides a handy set of editors to try.
          On a page with multiple copies of the editor (e.g. when adding a new multichoice question), an excessive number of database reads are seen.

          When the editor is inserted into a page, lib/form/editor.php::MoodleQuickForm_editor->toHtml() is called
          This in turn calls initialise_filepicker once for each type of media plugin - e.g. advimage, moodlemedia, and advlink
          The initialise_filepicker() function in turn calls repository::get_instances which generates a set of queries for each of the repository plugins.

          As an example on my default 2.1 install:
          {code}
          First editor: 57 queries
          Subsequent editors: 54 queries each
          Total queries for 17 editors = 57 + (54 * 16) = 921
          {code}

          Enabling an additional repository plugin (URL downloader) changes these numbers
          {code}
          First editor: 69 queries
          Subsequent editors: 66 queries each
          Total queries for 17 editors = 69 + (66 * 16) = 1125
          {code}
          (Note, I loaded the page twice after adding enabling additional repository plugins)

          We should see if it's possible to cache the results for some of these queries. They're mostly of the nature:
          {code}
          SELECT * FROM mdl_repository_instances WHERE id = $1 [array ( 0 => '1', )]
          SELECT * FROM mdl_repository_instance_config WHERE instanceid = $1 [array ( 0 => '1', )]
          SELECT * FROM mdl_repository WHERE id = $1 [array ( 0 => '1', )]
          SELECT name,value FROM mdl_config_plugins WHERE plugin = $1 [array ( 0 => 'local', )]
          {code}

          This is seen for any user (not just administrators). The question component is not at fault, but provides a handy set of editors to try.
          Hide
          Dan Poltawski added a comment -

          Adding Tim as a watcher here as he may have come across it

          Show
          Dan Poltawski added a comment - Adding Tim as a watcher here as he may have come across it
          Hide
          Tim Hunt added a comment -

          Ouch! I had not noticed that.

          Sounds solvable with some simple caching in repository class.

          Show
          Tim Hunt added a comment - Ouch! I had not noticed that. Sounds solvable with some simple caching in repository class.
          Hide
          Andrew Nicols added a comment -

          I've identified the key culprits and added some caching.

          The main one is the get_type_by_id() in is_visible. Caching this saves two queries per repository per media plugin, per editor.

          Caching the instance config in the repository constructor saved another two per repo, per media plugin, per editor.

          This took me down to just over 300 reads so there's still a significant amount of caching that can be done.

          Show
          Andrew Nicols added a comment - I've identified the key culprits and added some caching. The main one is the get_type_by_id() in is_visible. Caching this saves two queries per repository per media plugin, per editor. Caching the instance config in the repository constructor saved another two per repo, per media plugin, per editor. This took me down to just over 300 reads so there's still a significant amount of caching that can be done.
          Andrew Nicols made changes -
          Pull Master Diff URL https://git.luns.net.uk/?p=moodle.git;a=commitdiff;h=a05902999478ffe705e8a86bf9924fa1575638a8
          Pull Master Branch MDL-29831-master
          Pull from Repository git://git.luns.net.uk/moodle
          Labels editor performance
          Difficulty Moderate
          Hide
          Dan Poltawski added a comment -

          This is a real issue. It is making editing multichoice questions practically unusable for me, with the perf stats from produciton site:

          time: 66.553539s memory_total: 28413856B (27.1MB) memory_growth: 27638344B (26.4MB) memory_peak: 30130256B (28.7MB) includecount: 501 contextswithfilters: 0 filterscreated: 0 textsfiltered: 0 stringsfiltered: 0 langcountgetstring: 3216 langcountmemcache: 3041 langcountdiskcache: 183 includedyuimodules: 54 includedjsmodules: 4 db reads/writes: 4338/2 ticks: 6656 user: 291 sys: 35 cuser: 0 csys: 0 serverload: 0.00 Session: 57.9KB

          Show
          Dan Poltawski added a comment - This is a real issue. It is making editing multichoice questions practically unusable for me, with the perf stats from produciton site: time: 66.553539s memory_total: 28413856B (27.1MB) memory_growth: 27638344B (26.4MB) memory_peak: 30130256B (28.7MB) includecount: 501 contextswithfilters: 0 filterscreated: 0 textsfiltered: 0 stringsfiltered: 0 langcountgetstring: 3216 langcountmemcache: 3041 langcountdiskcache: 183 includedyuimodules: 54 includedjsmodules: 4 db reads/writes: 4338/2 ticks: 6656 user: 291 sys: 35 cuser: 0 csys: 0 serverload: 0.00 Session: 57.9KB
          Hide
          Dan Poltawski added a comment -

          Upgrading priority and adding performance component.

          Show
          Dan Poltawski added a comment - Upgrading priority and adding performance component.
          Dan Poltawski made changes -
          Priority Minor [ 4 ] Critical [ 2 ]
          Component/s Performance [ 10221 ]
          Hide
          Dan Poltawski added a comment -

          Urgh. I think more work than this needs to be done to fix it up properly.

          For example we are already retrieving the data which you are caching in the calling code

          Show
          Dan Poltawski added a comment - Urgh. I think more work than this needs to be done to fix it up properly. For example we are already retrieving the data which you are caching in the calling code
          Hide
          Dan Poltawski added a comment -

          There seem to be 36 calls to repository::get_instances() on that page

          Show
          Dan Poltawski added a comment - There seem to be 36 calls to repository::get_instances() on that page
          Hide
          Andrew Nicols added a comment - - edited

          IIRC, there are three TinyMCE plugins which use repositories. get_instances is called for each of these, and for each time the editor appears on screen.

          I'd guess that the page you're looking at has 12 TinyMCE editors on it, with three TinyMCE plugins each.

          Show
          Andrew Nicols added a comment - - edited IIRC, there are three TinyMCE plugins which use repositories. get_instances is called for each of these, and for each time the editor appears on screen. I'd guess that the page you're looking at has 12 TinyMCE editors on it, with three TinyMCE plugins each.
          Dan Poltawski made changes -
          Labels editor performance editor performance triaged
          Dan Poltawski made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Not knowing how complex it is... but I think the only correct solution to this is to delay the calculation of available repository instances to the exact moment they are requested (aka, press the add image/media... buttons, when the popup is shown).

          Anything else will require tricky caching, surely not covering all the situations.

          My 2 cents, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Not knowing how complex it is... but I think the only correct solution to this is to delay the calculation of available repository instances to the exact moment they are requested (aka, press the add image/media... buttons, when the popup is shown). Anything else will require tricky caching, surely not covering all the situations. My 2 cents, ciao
          Dan Poltawski made changes -
          Link This issue will be resolved by MDL-25290 [ MDL-25290 ]
          Hide
          Tony Levi added a comment -

          A couple of simple static caches work great for this problem.

          Show
          Tony Levi added a comment - A couple of simple static caches work great for this problem.
          Marina Glancy made changes -
          Link This issue has a non-specific relationship to MDL-34346 [ MDL-34346 ]
          Hide
          Andrew Nicols added a comment -

          Tony,

          You'll notice that this is the suggestion I made before, but there is an issue with this kind of static caching – although you may have multiple editors on a single page, they may have different repositories available to them.
          Catering for this can be difficult.

          Andrew

          Show
          Andrew Nicols added a comment - Tony, You'll notice that this is the suggestion I made before, but there is an issue with this kind of static caching – although you may have multiple editors on a single page, they may have different repositories available to them. Catering for this can be difficult. Andrew
          Hide
          Dan Poltawski added a comment -

          Andrew, you made a comment about this in dev chat. We are working on MUC (MDL-25290) for 2.4, please don't make any progress on this until we know where we are going with MUC as its a prime candidate.

          Show
          Dan Poltawski added a comment - Andrew, you made a comment about this in dev chat. We are working on MUC ( MDL-25290 ) for 2.4, please don't make any progress on this until we know where we are going with MUC as its a prime candidate.
          Michael de Raadt made changes -
          Link This issue will be resolved by MDL-36228 [ MDL-36228 ]
          Hide
          Tony Levi added a comment -

          Well, for interest sake, I found this in git which I think was related to this problem but it might be evil:

          function initialise_filepicker($args) {
          global $CFG, $USER, $PAGE, $OUTPUT;
          static $templatesinitialized;
          + static $cache = array();
          +
          + $key = serialize($args).$USER->id;
          + if (!empty($cache[$key])) return $cache[$key];
          +

          Show
          Tony Levi added a comment - Well, for interest sake, I found this in git which I think was related to this problem but it might be evil: function initialise_filepicker($args) { global $CFG, $USER, $PAGE, $OUTPUT; static $templatesinitialized; + static $cache = array(); + + $key = serialize($args).$USER->id; + if (!empty($cache [$key] )) return $cache [$key] ; +
          Marina Glancy made changes -
          Link This issue has a non-specific relationship to MDL-34346 [ MDL-34346 ]
          Marina Glancy made changes -
          Link This issue will be (partly) resolved by MDL-34346 [ MDL-34346 ]
          Hide
          Marina Glancy added a comment -

          This issue is resolved under MDL-34346

          Show
          Marina Glancy added a comment - This issue is resolved under MDL-34346
          Marina Glancy made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Resolution Duplicate [ 3 ]

            People

            • Votes:
              3 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: