Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed issue #CT-652: system slowness when saving survey settings #3740

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fixed issue #CT-652: system slowness when saving survey settings #3740

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7faa12b
Dev: increase performance when saving survey settings
ptelu Feb 14, 2024
5291b55
Dev: performance improvement when updating question attributes as par…
ptelu Feb 14, 2024
613983c
Merge branch 'master' into optimisations_general_settings
ptelu Feb 15, 2024
6477d12
Dev: revert static for attribute provider
ptelu Feb 15, 2024
5d81b58
Dev: code style
ptelu Feb 15, 2024
153c3b7
Dev: StartProcessingGroup requires gseq not gid
ptelu Feb 19, 2024
074e810
Dev: performance improvement when updating question attributes as par…
ptelu Feb 19, 2024
c5c093e
WIP gseq fixes
ptelu Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 22 additions & 5 deletions application/helpers/expressions/em_manager_helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8233,11 +8233,18 @@ public static function UpgradeQuestionAttributes($changeDB = false, $iSurveyID =
* @param int|null $surveyid
* @param int|null $qid
* @param string|null $lang
*
* @return array
* @throws CException
* @throws EmCacheException
*/
private function getQuestionAttributesForEM($surveyid = 0, $qid = 0, $lang = '')
{
$cacheKey = 'getQuestionAttributesForEM_' . $surveyid . '_' . $qid . '_' . $lang;
$gid = 0;
if (!empty($this->currentGroupSeq) && $this->currentGroupSeq > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be sure (if survey is set) LimeExpressionManager::getLEMsurveyId() == $surveyid

$gid = $this->currentGroupSeq;
}
$cacheKey = 'getQuestionAttributesForEM_' . $surveyid . '_' . $gid . '_' . $qid . '_' . $lang;
$value = EmCacheHelper::get($cacheKey);
if ($value !== false) {
return $value;
Expand Down Expand Up @@ -8277,14 +8284,24 @@ private function getQuestionAttributesForEM($surveyid = 0, $qid = 0, $lang = '')
'params' => [':qid' => $qid]
]
);
} elseif ($gid !== 0 && $surveyid) {
$oQids = Question::model()->findAll(
[
'select' => 'qid',
'group' => 'qid',
'distinct' => true,
'condition' => "sid=:sid and parent_qid=0 and gid=:gid",
'params' => [':sid' => $surveyid, ':gid' => $gid]
]
);
} elseif ($surveyid) {
$oQids = Question::model()->findAll(
[
'select' => 'qid',
'group' => 'qid',
'distinct' => true,
'select' => 'qid',
'group' => 'qid',
'distinct' => true,
'condition' => "sid=:sid and parent_qid=0",
'params' => [':sid' => $surveyid]
'params' => [':sid' => $surveyid]
]
);
} else {
Expand Down
13 changes: 11 additions & 2 deletions application/models/QuestionTheme.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,8 +835,16 @@ public static function getAnswerColumnDefinition($name, $type)
*/
public static function getQuestionXMLPathForBaseType($type)
{
/** @var QuestionTheme|null */
$questionTheme = QuestionTheme::model()->findByAttributes([], 'question_type = :question_type AND extends = :extends', ['question_type' => $type, 'extends' => '']);
static $questionThemeCache = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dislike to use Cache for Static var … Static is not caching ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah makes sense, cant use it everywhere php will run out of memory eventually when we use it to much. Will look at yii's caching

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently : it's a static var :). I don't ask to put it in cache (maybe in future), just more clear name :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also propose the creation of a protected static $questionThemeCache = []; data member to make the file more readable and the cache more accessible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected static $questionThemeStatic = [] ;)

if (!array_key_exists($type, $questionThemeCache)) {
$questionThemeCache[$type] = self::model()->findByAttributes(
[],
'question_type = :question_type AND extends = :extends',
['question_type' => $type, 'extends' => '']
);
}
$questionTheme = $questionThemeCache[$type];

if (empty($questionTheme)) {
throw new \CException("The Database definition for Questiontype: " . $type . " is missing");
}
Expand Down Expand Up @@ -1017,6 +1025,7 @@ public static function getAdditionalAttrFromExtendedTheme($sQuestionThemeName, $
if (\PHP_VERSION_ID < 80000) {
libxml_disable_entity_loader(false);
}

$questionTheme = QuestionTheme::model()->findByAttributes([], 'name = :name AND extends = :extends', ['name' => $sQuestionThemeName, 'extends' => $type]);
if ($questionTheme !== null) {
$xml_config = simplexml_load_file($questionTheme->getXmlPath() . '/config.xml');
Expand Down
2 changes: 2 additions & 0 deletions application/models/services/ThemeQuestionAttributeProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace LimeSurvey\Models\Services;

use QuestionTheme;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you use QuestionTheme here?


/**
* Provides question attribute definitions from question themes
*/
Expand Down