Moodle

• Type: Bug
• Status: Closed
• Priority: Critical
• Resolution: Fixed
• Affects Version/s: 1.8
• Fix Version/s: 1.8.3, 1.9
• Component/s: Course, Database activity module, Libraries, Roles / Access
• Labels:

  None

The function update_course_icon() in lib/weblib.php prints the course Turn editing on/off button. If a user does not have moodle/course:manageactivities or moodle/site:manageblocks at the course level, it then loops through every module, block and group instance in the course to check to see if the user has either of those capabilities in those contexts. This is where we run into performance issues because this section of code does not scale well. On top of that, after all that processing, the feature does not work. Example: give a student type user the admin role in the context of a block. The Turn editing on/off button will print, but you can never turn editing On.

Performance information:

Each child context that is checked generates roughly 11 database hits and this number increases if the course is nested below more than one category. Here is a breakdown:

1. update_course_icon() calls lib/accesslib.php:function get_child_contexts(). This method calls get_context_instance() for every block, module and group instance resulting in 1 db hit per instance. Instead of doing these calls individually, try to perform a batch process. Suggestion: new function get_context_instances(). This method could process an array and check the cache for any of the contexts and then make one query to get rest. It then caches result and returns an array of contexts. This would then do some looping and one db hit to get all child contexts instead of one db hit per child context.

2. in lib/weblib.php:function update_course_icon():
Replace:
$childcontext = get_record('context', 'id', $child);
With:
$childcontext = get_context_instance_by_id($child);

By using the API, we take advantage of the cache that was generated with the call to get_child_contexts(). This would reduce 1 db call per child context.

3. in lib/weblib.php:function update_course_icon():
Replace:
if (has_capability('moodle/course:manageactivities', $childcontext) ||
has_capability('moodle/site:manageblocks', $childcontext)) {
With:
if (has_capability('moodle/course:manageactivities', $childcontext, $USER->id, false) ||
has_capability('moodle/site:manageblocks', $childcontext, $USER->id, false)) {

If user had doanything, then s/he wouldn't be here in first place (I think). By turning off doanything, we reduce the db hits by 3 per child context.

4. in lib/accesslib.php:capability_search() - This method is called twice for every child context and generates 3 queries per call. This code should be evaluated for optimization and I don't have any suggestions at this time.

5. Lastly, the result of this (print edit button or not) should be saved in session so we don't have to do this again on the next page load.

Here are the db hit counts in the course I was testing this on:

• With core code: 21,930 database hits to process all the child contexts
• With code fixes 2 and 3 from above, database hits go down to 10,982.
• With code fixes 1, 2 and 3 hits would go down to 9,418
• Note: if you were to turn on db record cache at this point, the 9,418 would go down to approximately 1,581.

The majority of the remainder of the db hits (9,418) are coming from capability_search(). If this were to be optimized, then we could theoretically bring the db hits down to 100 or so total for the course page load (I think 60 or 80 db hits is normal for a course page load when not processing these child contexts).

That's all I have for now, good luck

1. 

   update_course_icon3.patch

   25/Aug/07 12:25 AM

   8 kB

   Petr Škoda (skodak)

2. 

   update_course_icon4.patch

   25/Aug/07 7:26 PM

   9 kB

   Petr Škoda (skodak)

3. 

   weblib_2.diff

   02/Aug/07 6:31 AM

   1 kB

   Mark Nielsen

4. 

   weblib.diff

   28/Jul/07 7:51 AM

   1 kB

   Mark Nielsen

• Assignee:

  Petr Škoda (skodak)

  Reporter:

  Mark Nielsen

  Tester:

  Nobody

  Participants:

  Mark Nielsen, Mark Pearson, Martin Dougiamas, Michael Penney, Petr Škoda (skodak)

• Created:

  28/Jul/07 7:46 AM

  Updated:

  17/Dec/10 1:08 PM

  Resolved:

  12/Sep/07 6:46 PM