View Issue Details

This bug affects 1 person(s).
 12
IDProjectCategoryView StatusLast Update
08308Bug reportsExpression Managerpublic2020-03-05 14:08
Reporterabezverkhyy Assigned ToLouisGac 
PriorityurgentSeverityminor 
Status closedResolutionfixed 
Product Version2.00+ 
Summary08308: questions that nothing depends upon trigger the reevaluation of every conditionnal expression in the survey
Description

In a survey with many conditional questions, in the user interacts with a question that nothing depends upon, every condition is reevaluated anyway.

Steps To Reproduce

Import the attached survey.
Take the survey.
On the first page choose the first answer.
Optionnal : enable a JS profiler
On the second page click on a any radio button
Optionnal : stop JS profiling
Result : even if you click on question that don't participate in any expression, thousands of calls to LEMrel and LEMval are performed.

Additional Information

It seems that in LEMrelXX functions, the UsesVars list isn't accurate, it is not empty for questions that nothing depends upon.

With Internet Explorer 8, any action on the attached survey takes more than 5 seconds.

TagsNo tags attached.
Attached Files
192_event.png (64,799 bytes)   
192_event.png (64,799 bytes)   
192_prof.png (71,549 bytes)   
192_prof.png (71,549 bytes)   
192_prof_tree2.png (76,870 bytes)   
192_prof_tree2.png (76,870 bytes)   
slow_survey_show_em.html (163,561 bytes)
Bug heat12
Complete LimeSurvey version number (& build)446139c3ad
I will donate to the project if issue is resolvedNo
Browser
Database type & versionnot relevant
Server OS (if known)not relevant
Webserver software & version (if known)not relevant
PHP Versionnot relevant

Users monitoring this issue

There are no users monitoring this issue.

Activities

c_schmitz

c_schmitz

2013-10-24 01:34

administrator   ~26971

Thomas, do you have a quick idea on this?

TMSWhite

TMSWhite

2013-10-24 01:40

reporter   ~26972

Does the same problem happen in 1.92? To the best of my knowledge, the LEMrelXX functions were specifically designed to only be called for questions that have some dependency upon others - whether in relevance, validation, or tailoring. There were some limitations - such as questions having array filters, that had to always be called, but beyond that, I did try to minimize the number of calls.

I don't have an easy way to test -- other than a production website, I haven't done anything with LimeSurvey in over a year, and don't have a computer on which it is installed.

c_schmitz

c_schmitz

2013-10-24 07:55

administrator   ~26975

abezverkhyy, does this happen with 2.0 or 1.92, too?

abezverkhyy

abezverkhyy

2013-10-24 13:08

reporter   ~26979

Exact same behavior on LimeSurvey 1.92. See the screenshots attached.

TMSWhite

TMSWhite

2013-10-24 13:38

reporter   ~26980

Please also attach a screen-shot of the "Show Logic File" for those two questions. It appears that Q7 uses array_filter from the previous question to specify which sub-questions should appear. I found that it was necessary to call check_conditions for all sub-entries within array_filter in order to properly handle cascading array_filter (cases where Q3 was filtered on Q2 and Q2 was filtered on Q1).

So, although it might be possible to reduce the number of calls to check_conditions, doing so in this case could break the accuracy of the conditional logic.

Two suggestions:
(1) Although the debugger shows 1.5 seconds for each call, it will be faster wit the debugger off.
(2) Internet Explorer 8 is notoriously slow with LimeSurvey. Chrome and Mozilla both run 5-10 times faster than Internet Explorer 8 on the same survey.

abezverkhyy

abezverkhyy

2013-10-25 11:54

reporter   ~26988

I just attached the output from "Generate Logic File". Notice that nothing uses Q7 in its expression, but any event on Q7 trigger thousands of reevaluations.

(1) this is not true, with or without the profiler, I can easily count to four seconds before any form element reacts, and I have a pretty good machine, imagine what it might be on low-end hardware.

(2) I agree, but you don't usually know the people who will take your survey and you can't force them to install browser X or Y, sadly IE8 is still used by something like 10% of Internet users.

I understand that some events might always remain slow on IE8 as its JS engine lacks performance, but what I'm pointing out in this bug report is that the generated JS code might be doing lots of useless reevaluations which is an algorithmic issue.

c_schmitz

c_schmitz

2013-10-30 08:45

administrator   ~27023

Last edited: 2013-10-30 08:46

The source of the issue is that the question group has a condition. A group condition is bool-added to the condition of each question and re-evaluated on each action.

TMSWhite, do you remember why you did that? If a group is already displaying I think it makes not much sense to re-evaluate it over and over in each action inside the group?

A workaround might be to have smaller groups. But I did not check that.

TMSWhite

TMSWhite

2013-10-31 01:34

reporter   ~27048

I believe that was needed to ensure accuracy in cascading array filter exclude (bug #06087). You can test this using some of the demosurveys, like https://github.com/LimeSurvey/LimeSurvey/blob/master/docs/demosurveys/ls2_cascading_array_filter_exclude.lss

c_schmitz

c_schmitz

2013-11-20 12:05

administrator   ~27246

I think that in group-by-group or all-in-one mode it would have been better to create a group container element that is under EM control.

That would even cover the twisted 'group condition base on a question inside its group' scenario and could be applied even to question-by-question mode.

c_schmitz

c_schmitz

2016-02-15 15:51

administrator   ~35149

IE11 still is somewhat slow but alot better.

LouisGac

LouisGac

2016-08-24 18:35

developer   ~40471

for further references, the UsesVars list is generated here in this loop:

https://github.com/LimeSurvey/LimeSurvey/blob/master/application/helpers/expressions/em_manager_helper.php#L7205-L7325

to be more precise, here:
https://github.com/LimeSurvey/LimeSurvey/blob/master/application/helpers/expressions/em_manager_helper.php#L7317

and seems correct

LouisGac

LouisGac

2016-08-24 19:04

developer   ~40472

First impressions:

I can be wrong, but it seems that the problem is this one:

in survey_runtime.js, function checkconditions we have:

if($.isFunction(window.ExprMgr_process_relevance_and_tailoring ))
{        
    ExprMgr_process_relevance_and_tailoring(evt_type,name,type);
}

The function checkconditions is called for each "onchange" event of each question.

If one question has relevance, the function ExprMgr_process_relevance_and_tailoring will exist, so the condition will be true for all questions, so it's called each time you click on a question, whatever is the question...

So, a way to fix this would be to replace the condition by something like:

if (any question depends on the question clicked)
then
ExprMgr_process_relevance_and_tailoring

LouisGac

LouisGac

2016-08-24 20:19

developer   ~40474

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=20218

c_schmitz

c_schmitz

2016-08-25 09:50

administrator   ~40479

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=20220

LouisGac

LouisGac

2016-08-25 11:21

developer   ~40483

Fix committed to master branch: https://github.com/LimeSurvey/LimeSurvey/commit/f48151539a7bc5ad5cc012a9f446981b51b7374e

ollehar

ollehar

2016-10-20 15:01

administrator   ~41500

Isn't this one fixed?

LouisGac

LouisGac

2016-10-20 15:10

developer   ~41504

I have to do the last part of it.
I will do it next week.

LouisGac

LouisGac

2016-11-14 17:09

developer   ~41919

bump

LouisGac

LouisGac

2017-11-17 18:58

developer   ~45107

Fix committed to develop branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=24910

ollehar

ollehar

2017-12-01 12:31

administrator   ~45215

Looks like this fix causes this regression: https://bugs.limesurvey.org/view.php?id=12980

LouisGac

LouisGac

2017-12-01 12:32

developer   ~45216

yep

markusfluer

markusfluer

2018-05-28 17:56

administrator   ~47882

Small amendment, we had to exclude equation questions from the filtering.
This is because it may be that an equation question is directly dependant on another equation question.
Due to the selective reevaluation this leads to the issue that the second line equation question is not reevaluated and thus stays at the old value.
This has been done in commit a9daeeabbdaeaacaa19f0c94f7ed60b00a3b7691

Related Changesets

LimeSurvey: master 317b1f20

2016-08-24 18:19:00

LouisGac

Details Diff
Fixed issue 08308: questions that nothing depends upon trigger the reevaluation of every conditionnal expression in the survey
Dev: added a check for question condition
Affected Issues
08308
mod - application/helpers/SurveyRuntimeHelper.php Diff File
mod - application/models/Condition.php Diff File
mod - scripts/survey_runtime.js Diff File

LimeSurvey: master 435d551f

2016-08-25 07:50:37

c_schmitz

Details Diff
Fixed issue 08308: questions that nothing depends upon trigger the reevaluation of every conditionnal expression in the survey
Dev Small fix if no conditions exist
Affected Issues
08308
mod - application/helpers/SurveyRuntimeHelper.php Diff File

LimeSurvey: develop adf09d2d

2017-11-17 18:57:47

LouisGac

Details Diff
Fixed issue 08308: questions that nothing depends upon trigger the reevaluation of every conditionnal expression in the survey Affected Issues
08308
mod - application/config/version.php Diff File
mod - application/helpers/expressions/em_manager_helper.php Diff File
mod - assets/scripts/survey_runtime.js Diff File

Issue History

Date Modified Username Field Change
2013-10-22 16:15 abezverkhyy New Issue
2013-10-22 16:15 abezverkhyy File Added: limesurvey_survey_124884__analyse_bug_lenteur.lss
2013-10-24 01:34 c_schmitz Assigned To => TMSWhite
2013-10-24 01:34 c_schmitz Status new => assigned
2013-10-24 01:34 c_schmitz Note Added: 26971
2013-10-24 01:40 TMSWhite Note Added: 26972
2013-10-24 07:55 c_schmitz Note Added: 26975
2013-10-24 07:56 c_schmitz Assigned To TMSWhite => c_schmitz
2013-10-24 07:56 c_schmitz Status assigned => feedback
2013-10-24 13:07 abezverkhyy File Added: 192_event.png
2013-10-24 13:07 abezverkhyy File Added: 192_prof.png
2013-10-24 13:07 abezverkhyy File Added: 192_prof_tree2.png
2013-10-24 13:08 abezverkhyy Note Added: 26979
2013-10-24 13:08 abezverkhyy Status feedback => assigned
2013-10-24 13:38 TMSWhite Note Added: 26980
2013-10-25 11:38 abezverkhyy File Added: slow_survey_show_em.html
2013-10-25 11:54 abezverkhyy Note Added: 26988
2013-10-30 08:45 c_schmitz Note Added: 27023
2013-10-30 08:45 c_schmitz Assigned To c_schmitz => TMSWhite
2013-10-30 08:45 c_schmitz Status assigned => feedback
2013-10-30 08:46 c_schmitz Note Edited: 27023
2013-10-31 01:34 TMSWhite Note Added: 27048
2013-11-07 15:05 c_schmitz Assigned To TMSWhite => c_schmitz
2013-11-07 15:05 c_schmitz Status feedback => assigned
2013-11-20 12:05 c_schmitz Note Added: 27246
2013-11-20 12:24 c_schmitz Target Version => 2.10
2013-11-20 12:26 c_schmitz Assigned To c_schmitz =>
2013-11-20 12:26 c_schmitz Status assigned => acknowledged
2016-02-15 15:51 c_schmitz Note Added: 35149
2016-08-24 18:35 LouisGac Note Added: 40471
2016-08-24 19:04 LouisGac Note Added: 40472
2016-08-24 20:19 LouisGac Changeset attached => LimeSurvey master 317b1f20
2016-08-24 20:19 LouisGac Note Added: 40474
2016-08-24 20:19 LouisGac Assigned To => LouisGac
2016-08-24 20:19 LouisGac Resolution open => fixed
2016-08-25 09:50 c_schmitz Changeset attached => LimeSurvey master 435d551f
2016-08-25 09:50 c_schmitz Note Added: 40479
2016-08-25 09:50 c_schmitz Assigned To LouisGac => c_schmitz
2016-08-25 11:21 LouisGac Note Added: 40483
2016-09-21 15:16 c_schmitz Assigned To c_schmitz => LouisGac
2016-09-21 15:17 c_schmitz Status acknowledged => assigned
2016-10-20 15:01 ollehar Note Added: 41500
2016-10-20 15:10 LouisGac Note Added: 41504
2016-11-14 17:09 LouisGac Note Added: 41919
2016-11-14 17:10 LouisGac Priority normal => urgent
2017-11-16 17:52 LouisGac Sticky Issue No => Yes
2017-11-17 18:58 LouisGac Changeset attached => LimeSurvey develop adf09d2d
2017-11-17 18:58 LouisGac Note Added: 45107
2017-12-01 12:31 ollehar Note Added: 45215
2017-12-01 12:32 LouisGac Note Added: 45216
2018-02-22 14:18 c_schmitz Sticky Issue Yes => No
2018-05-28 17:56 markusfluer Note Added: 47882
2020-03-05 14:08 cdorin Status assigned => closed