CTANZ / moodle-theme_decaf

Moodle 2 theme - decaf
https://github.com/leizhang/moodle-theme_decaf/wiki/
17 stars 17 forks source link

Performance problem with bigger Moodle installation #4

Open mangus opened 11 years ago

mangus commented 11 years ago

Hi!

Very nice theme, but we have a performance problem in our Moodle installation (~6500 courses, ~500 course categories).

When guest user accesses the course page, then the top menu wants to show all courses from Moodle. That makes the page run out of PHP maximum execution time (30 seconds). Also the Javascript, that handles the top menu will make the browser freeze for about 40 seconds (when I increase the PHP _max_executiontime parameter).

Fortunately there is a theme setting coursesloggedinonly, but this still adds courses for the quest user.

So i suggest that this setting would also hide courses for the guest user by chancing renderers.php line 283:

$hidecourses = (property_exists($PAGE->theme->settings, 'coursesloggedinonly') && $PAGE->theme->settings->coursesloggedinonly && !isloggedin());

to

$hidecourses = (property_exists($PAGE->theme->settings, 'coursesloggedinonly') && $PAGE->theme->settings->coursesloggedinonly && (!isloggedin() || isguestuser()));

Anyway, thanks for a great theme!

pauln commented 11 years ago

Hi Mart, Thanks for the suggestion. Are you / your admin(s) also experiencing issues due to the number of courses, or do all of your real (i.e. non-guest) users have enrolments (which cause "My courses" to take over from "Courses", dodging this issue)?

I'm contemplating adding an option to remove everything under "Courses", so there's still an easy way to get to the list of courses - but without the overhead of adding them all to the menu. What do you think?

mangus commented 11 years ago

Thanks for quick response!

Other users (non-guests) have enrollments and guest accounts where the only place where we hit this problem.

Your solution with theme setting sounds good!

mangus commented 11 years ago

Hi again,

just a little note, that selecting the Decaf theme as default theme will make this problem appear in front page for all not authenticated users.

Anyway, the solution You offered solves this one also.

danmarsden commented 11 years ago

Hey Paul - it would be really good if we could have a setting that allowed "courses" node to be completely removed including this in the header causes a massive perf hit on larger sites.

danmarsden commented 11 years ago

looks like this could be done by renaming the coursesloggedinonly setting and allow it to have multiple options: Never Always LoggedinOnly

Then modifying this line in decaf/renderer.php with the new options:

$hidecourses = (property_exists($PAGE->theme->settings, 'coursesloggedinonly') && $PAGE->theme->settings->coursesloggedinonly && !isloggedin());

pauln commented 11 years ago

Hi Dan, Have you tried setting the 'Populate "Courses" menu' setting to "No" in Decaf's settings? It leaves the "courses" node there, so you can get to the list of courses easily without needing the course list block - but skips actually populating it. If it's not doing the trick for you, please double-check you're using the Decaf v1.8.x (the settings was added in 1.8) and then let me know which version of Moodle you're running it on; it seems to do the trick for us on our 2.4.1-based build...

danmarsden commented 11 years ago

ahh - yes, we're using an older theme there so I didn't have that setting - trying it now.

one of the sites I'm using creates over 2400Db queries to build that "courses" list in the decaf theme - must be some weird recursive code in there somewhere... it shouldn't take more than 1 or 2 queries to get that list....

pauln commented 11 years ago

Yeah, I think I've had a bit of a poke around but couldn't immediately see why the number of queries skyrockets like that - so I took the quick and easy approach of just not generating the list at all. If you're able to figure it out, I'd be more than happy to accept a patch to fix it!

danmarsden commented 11 years ago

yeah - that works for me (new setting) - I didn't get a chance to trace exactly where the dodgy stuff was occurring - although I'm just about to do some perf testing for users with large numbers of enrolments.

pauln commented 11 years ago

Dan (and anyone else who's interested), I've just pushed a new branch "awesomebar-performance", which brings the number of DB queries back down to normal levels and also makes another performance boost for the courses menu (using a combination of tree traversal and ::get() instead of ::find(), since ::find() is terribly slow for deep nodes). If you can try it out, please let me know how you get on; my dev server was still considerably slower generating pages with the full courses menu than without, but it was taking a couple of seconds rather than the 10+ that it was taking without these changes.

danmarsden commented 11 years ago

nice - that looks like the worst offender! https://github.com/CTANZ/moodle-theme_decaf/commit/f469c6006794845a343502e986c61eb0f4a0338b

I won't have time to check this myself this week though sorry!