sensei icon indicating copy to clipboard operation
sensei copied to clipboard

Performance issue for Co-Teachers on site with Sensei Pro and many learners

Open markcummins opened this issue 4 months ago • 0 comments

Steps to Reproduce

  1. Create a site with many learners with the Sensei Pro Plugin enabled
  2. Log in as a co-teacher

What I Expected

We noticed that our site was very slow for our co-teacers. Average load time was about 2 seconds slower, although our site has qulte a lot of learners (around 9,000)

PHP / WordPress / Sensei LMS version

Sensei Version 4.25.0 Sensei Pro Version Version 1.24.3 Wordpress Version 6.8.2 PHP version 8.2.28

Main problem

The main problem is that the get_teacher_courses() function is called, which then calls the Sensei()->course->get_all_courses function. This in itself is a bit slow. But it sets up the restore_module_terms_for_coteachers filter in sensei-pro/modules/co-teachers/includes/class-co-teachers-permissions.php which is the main cause of the issue.

/**
 * Iterates over all the original terms and adds back the terms for modules that are used in a Course where current user is a co-teacher.
 *
 * @param \WP_Term[]       $terms          The final terms after removing the unowned terms.
 * @param \WP_Term[]|int[] $original_terms The original list of all the terms previous to filtering.
 * @return array
 */
public function restore_module_terms_for_coteachers($terms, $original_terms)
{
	$coauthor_terms = [];

	foreach ($original_terms as $original_term) {
		if (is_numeric($original_term)) {
			// The term id was given, get the term object.
			$original_term = get_term($original_term, 'module');
		}

		$object_ids = get_objects_in_term($original_term->term_id, 'module');
		foreach ($object_ids as $post_id) {
			$post = get_post($post_id);
			if ($this->co_teachers->is_coteacher(get_current_user_id(), $post)) {
				$coauthor_terms[] = $original_term;
				break; // Continue to next `$original_term`.
			}
		}
	}

	return array_merge($terms, $coauthor_terms);
}

The $original_terms variable is bringing back not just module terms, but also sensei_learner terms too. We have lots of learners, so the loop will call the get_objects_in_term around 9000 times.

Short term fix

You could add a check there so it would skip over the 'sensei_learner' terms. For me, that reduced the time of the function from 2 seconds to 0.25.

foreach ($original_terms as $original_term) {
	if (is_numeric($original_term)) {
		// The term id was given, get the term object.
		$original_term = get_term($original_term, 'module');
	}

	// Ignore terms that are not modules.
	if ($original_term->taxonomy !== 'module') {
		continue;
	}

	$object_ids = get_objects_in_term($original_term->term_id, 'module');
	foreach ($object_ids as $post_id) {
		$post = get_post($post_id);
		if ($this->co_teachers->is_coteacher(get_current_user_id(), $post)) {
			$coauthor_terms[] = $original_term;
			break; // Continue to next `$original_term`.
		}
	}
}

Call Stack

Image

markcummins avatar Aug 14 '25 15:08 markcummins

Thank you very much @bidzanaaron !

This looks good as far as I can see, ...but will be hell to pick to newer versions.

One thing: Could you please change the nomenclature in ilAssQuestionFeedback::syncPageObject()? I think the function should be ilAssQuestionFeedback::copyPageObject($source, $target): Synchronizing is very unclear in its meaning. You can synchronize in any direction or both, so it is very hard to know what is meant by it. So "copy" from "source" to "target" should be correct.

This can then be merged.

Thanks and best, @kergomard

kergomard avatar Dec 01 '25 09:12 kergomard

Hey @kergomard,

I have changed the method signature and variables based on your feedback.

Kind regards, @bidzanaaron

bidzanaaron avatar Dec 01 '25 13:12 bidzanaaron

ILIAS 10 PR for this: #10698

Code now okay @kergomard?

dsstrassner avatar Dec 12 '25 09:12 dsstrassner

Hey @bidzanaaron,

thanks for the PR. I've integrated the changes into release_9.

Best Regards, @thojou

thojou avatar Dec 15 '25 13:12 thojou