Moodle

course/scales.php reallllllllly slow

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Trivial Trivial
  • Resolution: Fixed
  • Affects Version/s: 1.4.1
  • Fix Version/s: 1.8.3, 1.9
  • Component/s: Administration
  • Labels:
    None
  • Environment:
    All
  • Affected Branches:
    MOODLE_14_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE

Description

The scales.php script checks all the site wide custom scales against every course in the database to check if its used. This really slows down the page.

This checking is not necessary unless the person accessing the scales.php page is the administrator, since it is only the administrator who can edit site wide scales.

The version i am using is Moodle 1.4 development (2004070800)

  1. scales_perf_18_v2.patch
    24/Sep/07 2:36 PM
    13 kB
    Samuli Karevaara
  2. scales_perf_18.patch
    20/Sep/07 8:10 PM
    10 kB
    Samuli Karevaara

Activity

Hide
Martin Dougiamas added a comment -

From Girishan Shanmugam (girishan at tamu.edu) Friday, 3 September 2004, 11:49 PM:

I made this quick change that makes the page faster for non admins in the /course/scales.php script.

439,444c439

< //A change to speed up the scales pages for non admins

< //September 3rd 2004. Girishan Shanmugam

< if(isadmin()){ < $scales_uses = site_scale_used($scale->id); < }

< //End of change.

> $scales_uses = site_scale_used($scale->id);

From Girishan Shanmugam (girishan at tamu.edu) Saturday, 4 September 2004, 12:17 AM:

I took the diff the wrong way. This should make more sense. (not yet familiar with diff/patch)

File: /course/scales.php

439c439,444

< $scales_uses = site_scale_used($scale->id);

> //A change to speed up the scales pages for non admins

> //September 3rd 2004. Girishan Shanmugam

> if(isadmin()){ > $scales_uses = site_scale_used($scale->id); > }

> //End of change.

From Martin Langhoff (martin at catalyst.net.nz) Monday, 13 December 2004, 04:13 PM:

girishan, 1.4.2+ contain some performance improvements to the scales area. Can you give it a try and report back?

Show
Martin Dougiamas added a comment - From Girishan Shanmugam (girishan at tamu.edu) Friday, 3 September 2004, 11:49 PM: I made this quick change that makes the page faster for non admins in the /course/scales.php script. 439,444c439 < //A change to speed up the scales pages for non admins < //September 3rd 2004. Girishan Shanmugam < if(isadmin()){ < $scales_uses = site_scale_used($scale->id); < } < //End of change. — > $scales_uses = site_scale_used($scale->id); From Girishan Shanmugam (girishan at tamu.edu) Saturday, 4 September 2004, 12:17 AM: I took the diff the wrong way. This should make more sense. (not yet familiar with diff/patch) File: /course/scales.php 439c439,444 < $scales_uses = site_scale_used($scale->id); — > //A change to speed up the scales pages for non admins > //September 3rd 2004. Girishan Shanmugam > if(isadmin()){ > $scales_uses = site_scale_used($scale->id); > } > //End of change. From Martin Langhoff (martin at catalyst.net.nz) Monday, 13 December 2004, 04:13 PM: girishan, 1.4.2+ contain some performance improvements to the scales area. Can you give it a try and report back?
Hide
Wen Hao Chuang added a comment -

this is a REALLY old ticket should we close this one please?

Show
Wen Hao Chuang added a comment - this is a REALLY old ticket should we close this one please?
Hide
Samuli Karevaara added a comment -

Don't close this issue yet! The /course/scales.php takes about a minute on our site, with 1.8.2+.

Show
Samuli Karevaara added a comment - Don't close this issue yet! The /course/scales.php takes about a minute on our site, with 1.8.2+.
Hide
Samuli Karevaara added a comment -

One culprit is the site_scale_used(), it is called for every scale and it walks through every course, and for every course it walks through every course module, and for every one of them it calls the $cm->modname.'_scale_used' just to return the number of activities using the scale in question. Did I emphasize the every's enough?

The custom functions for each module are a tough one, otherwise a single query similar to SELECT COUNT SOMETHINGSOMETHING WHERE scaleid = $scaleid AND course = $courseid would do just fine.

Show
Samuli Karevaara added a comment - One culprit is the site_scale_used(), it is called for every scale and it walks through every course, and for every course it walks through every course module, and for every one of them it calls the $cm->modname.'_scale_used' just to return the number of activities using the scale in question. Did I emphasize the every's enough? The custom functions for each module are a tough one, otherwise a single query similar to SELECT COUNT SOMETHINGSOMETHING WHERE scaleid = $scaleid AND course = $courseid would do just fine.
Hide
Samuli Karevaara added a comment -

Correcting myself: not for every scale, but for every global scale (with courseid = 0). Also, I didn't mean that all the site modules are scanned times the courses amount, it walks through the courses own modules. Our site gives 33K queries with just two global scales, so this function can behave a bit badly.

Show
Samuli Karevaara added a comment - Correcting myself: not for every scale, but for every global scale (with courseid = 0). Also, I didn't mean that all the site modules are scanned times the courses amount, it walks through the courses own modules. Our site gives 33K queries with just two global scales, so this function can behave a bit badly.
Hide
Martin Dougiamas added a comment -

Petr, I see get_item_uses_count() in the new gradebook still uses this code (and there is a TODO there) ... can you think of a better way?

We don't really need a count, we just need a Boolean result (to prevent deleting) ... as soon as we know it's used somewhere we should stop looking.

Show
Martin Dougiamas added a comment - Petr, I see get_item_uses_count() in the new gradebook still uses this code (and there is a TODO there) ... can you think of a better way? We don't really need a count, we just need a Boolean result (to prevent deleting) ... as soon as we know it's used somewhere we should stop looking.
Hide
Petr Škoda (skodak) added a comment -

fixed in cvs, I had to add a new function to modules to do the scale used check across all courses - I could not find a backwards compatible way to reuse the old one that was designed primarily for backup purposes. Updated the NEWMODULE template in contrib too.

Show
Petr Škoda (skodak) added a comment - fixed in cvs, I had to add a new function to modules to do the scale used check across all courses - I could not find a backwards compatible way to reuse the old one that was designed primarily for backup purposes. Updated the NEWMODULE template in contrib too.
Hide
Petr Škoda (skodak) added a comment -

if there is any 3rd party unpatched mod, the old reallllly slow code is still used.

Show
Petr Škoda (skodak) added a comment - if there is any 3rd party unpatched mod, the old reallllly slow code is still used.
Hide
Petr Škoda (skodak) added a comment -

adding some info for developers when this new function not found:
debugging('Please notify the developer of module "'.$mod->name.'" that new function module_scale_used_anywhere() should be implemented.', DEBUG_DEVELOPER);

Show
Petr Škoda (skodak) added a comment - adding some info for developers when this new function not found: debugging('Please notify the developer of module "'.$mod->name.'" that new function module_scale_used_anywhere() should be implemented.', DEBUG_DEVELOPER);
Hide
Samuli Karevaara added a comment -

Here is a patch for 1.8 series. It's pretty similar to what Petr did for 1.9. If I get a change to test it a bit more couldI check it in 1.8?

Show
Samuli Karevaara added a comment - Here is a patch for 1.8 series. It's pretty similar to what Petr did for 1.9. If I get a change to test it a bit more couldI check it in 1.8?
Hide
Samuli Karevaara added a comment -

After correcting the patch a bit (attaching a new version here) and running it on our largest production site (2,500 courses, 11,000 users) I checked the fix in 1.8. I didn't want to add new text strings here, so instead of "Activities: N" the strings now say just "Activities: Yes" which I think is informative also. The main purpose of the message is to inform that the activity is in use and thus can't be edited.

Show
Samuli Karevaara added a comment - After correcting the patch a bit (attaching a new version here) and running it on our largest production site (2,500 courses, 11,000 users) I checked the fix in 1.8. I didn't want to add new text strings here, so instead of "Activities: N" the strings now say just "Activities: Yes" which I think is informative also. The main purpose of the message is to inform that the activity is in use and thus can't be edited.

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: