Skip to content

Commit

Permalink
[Performance] Do not load module logger from constructor
Browse files Browse the repository at this point in the history
This is an attempt to solve the speed issues with demo-24.loris.ca.

In LORIS 23, there was no LORISInstance object that was used for
getting active modules, nor was there a logger instantiated from the
constructor. The added overhead of retrieving the log settings for
every module instantiation and LorisInstance->getActiveModules
instantiating every module is adding up to a lot of overhead in
the critical path on demo-24.loris.ca.

This moves the logger instantiation to be done after the
Module::factory call in the router instead of in the constructor.
The result is that it's only done once, instead of done n times
(where n is the number of modules.)

This doesn't get demo-24.loris.ca up to the speed of demo.loris.ca,
but it's significantly closer.

It's not clear why this performance regression is only happening on
demo and not in other environments.
  • Loading branch information
driusan committed Oct 25, 2022
1 parent 81e2588 commit 5d3d573
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
4 changes: 0 additions & 4 deletions php/libraries/Module.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,6 @@ abstract class Module extends \LORIS\Router\PrefixRouter
*/
public function __construct(string $name, string $moduledir)
{
$config = \NDB_Factory::singleton()->config();
$loglevel = $config->getLogSettings()->getRequestLogLevel();

$this->logger = new \LORIS\Log\ErrorLogLogger($loglevel);
parent::__construct(
new \ArrayIterator(
[
Expand Down
21 changes: 14 additions & 7 deletions src/Router/BaseRouter.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,12 @@ public function handle(ServerRequestInterface $request) : ResponseInterface

$factory = \NDB_Factory::singleton();
$ehandler = new \LORIS\Middleware\ExceptionHandlingMiddleware();
$exceptionloglevel = $this->lorisinstance->getConfiguration()
->getLogSettings()
->getExceptionLogLevel();
$logSettings = $this->lorisinstance->getConfiguration()->getLogSettings();
$exceptionloglevel = $logSettings->getExceptionLogLevel();

if ($exceptionloglevel != "none") {
$ehandler->setLogger(
new \LORIS\Log\ErrorLogLogger(
$factory->config()->getLogSettings()->getExceptionLogLevel()
)
new \LORIS\Log\ErrorLogLogger($exceptionloglevel)
);
} else {
$ehandler->setLogger(new \PSR\Log\NullLogger);
Expand All @@ -120,7 +117,17 @@ public function handle(ServerRequestInterface $request) : ResponseInterface

$factory->setBaseURL($baseurl);

$module = \Module::factory($modulename);
$module = \Module::factory($modulename);

$requestloglevel = $logSettings->getRequestLogLevel();
if ($requestloglevel != "none") {
$module->setLogger(
new \LORIS\Log\ErrorLogLogger($requestloglevel)
);
} else {
$module->setLogger(new \PSR\Log\NullLogger);
}

$mr = new ModuleRouter($module);
$request = $request->withURI($suburi);
return $ehandler->process($request, $mr);
Expand Down

0 comments on commit 5d3d573

Please # to comment.