jonof / moodle-block_completion_progress

A time management tool for students using activity completion
https://moodle.org/plugins/block_completion_progress
GNU General Public License v3.0
17 stars 65 forks source link

mod_assign grading page never loads due to a big number of Completion progress blocks #83

Closed golenkovm closed 2 years ago

golenkovm commented 2 years ago

Hi,

In a Moodle 3.9 site with 8,000 Completions progress block instances teachers can't open assignment grading page. The /lib/ajax/service.php?sesskey=xxx&info=core_get_fragment' AJAX call takes ages to complete.

As I can see on_site_page() returns true https://github.com/jonof/moodle-block_completion_progress/blob/0772069a24b132ac585f7b98706edef6d0a0a62c/block_completion_progress.php#L333 because$page->pagetype is set to site-index for this call. Thus, get_content() iterates through all teacher's courses and then through all course blocks.

golenkovm commented 2 years ago

Please find profiling for the request below:

image

It shows how the number of calls jumps from 9 prepare_dashboard_content() up to 5k for_block_instance().

golenkovm commented 2 years ago

A side note: some blocks have Display on page types set to Any page, so they are picked up by $OUTPUT->blocks('side-pre'); and there fore there is a call to get_content().

jonof commented 2 years ago

Thanks for the profiling trace. I've revised the site/non-site-page test to use page context level instead of taking guesses from page type values, which hindsight says I probably should have done when I changed it 11 months ago. The way I'm sniffing CLI/PHPUnit gives me uneasiness, so I'm going to mull on that a little more. We may also be able to optimise away entirely a wasted block render with this:

--- a/block_completion_progress.php
+++ b/block_completion_progress.php
@@ -117,6 +117,9 @@ class block_completion_progress extends block_base {
             return $this->content;
         }

+        if (AJAX_SCRIPT) {
+            return $this->content;
+        }
         if (self::on_site_page($this->page)) {
             // Draw the multi-bar content for the Dashboard and Front page.
             if (!$this->prepare_dashboard_content($barinstances)) {
golenkovm commented 2 years ago

Thank you @jonof I haven't tested your patch yet, but the code looks good. Cheers

golenkovm commented 2 years ago

Hi @jonof I just deployed the recent code and gave this a test. It seems like the patch works as expected and the issue if fully fixed. Thank you.