-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
Performance Issue #11
Comments
Hi Kathrin, thanks for reporting this. We forwarded the problem to the developer to see if he sees a realistic possibility to improve the performance. Best regards, |
H i Kathrin, Thanks. |
Hi Kathrin, |
no further information since last year, therefor we are closing the issue. |
Hi Kasia, I also would like to add my two cents. I expect that there may be somewhere in the code a for/foreach-loop which interates on each slot (to be created / deleted) and fires one database query for each slot. This way, adding / deleting more slots at a time will directly and proportionately result in more database connections and queries which might be unnoticeable on development systems and quick DBs, but might have a impact on big / slower DBs like ours. I haven't searched the code if this expectation is really true and I may be talking bullshit here, but Kathrin's performance research data gives hints that it could be something like that. I would be grateful if you could examine your code for a leak like that or do your own performance research and try to improve it for example by adding / deleting multiple slots in one DB query. Thanks, |
Hi Kathrin, Hi Alex, |
Hi Kasia, you're welcome. Thanks, |
Dear Kasia, On the behalf of Alex and Kathrin, I had a look on this issue as well. I noticed that one reason for this issue might be the way in which appointment_slots are added to the respective table and the events table (lines 169-172 in locallib.php). There are two insert and one update query that are sent to the db for each slot. After the first bulk insert (into the orginizer_slots table) I selected the new records using this statement: $conditions = array('organizerid' => $organizer->id, 'eventid' => null); Using the ids of the added slots i could bulk insert the events as well. Then I used the following updated statement: $subquery = "SELECT id FROM {event} AS e WHERE {organizer_slots}.id = e.instance AND {organizer_slots}.timemodified = e.timemodified AND {organizer_slots}.starttime = e.timestart"; (As one might see the way in which added organizer_slots are determined and the $updatequery both are not completely safe. The former simply considers any slot of the same organizer that has not been connected to an event yet and may include legacy slots or also return slots that are created by a concurrent action if transactions are not used. The latter simply connects slots with events of the same starttime, timemodified and organizer_slot_id as instanceid - if there is an event with the same starttime, timemodified and instanceid (that is not an orgnizer_slot_id but refers to any other module) then the query my corrupt the table. However, both problems dan be be improved by using or introducing more unique identifiers/comparisons) But, I have to admit that this solution does not solve this issue completely and there is at least one other reason that consumes much time. I assume it might be the the final presentation of the added slots as all slots are shown instead of the fix number of slots (e.g. first 20), but I did not find any more time to verify this. Nevertheless, I hope that my remark might be helpful if one wants to improve the performance of the organizer with respect to this issue. Cheers, |
Hi David, thank you very much for putting this additional information together — I added your new information to our issue and hope it will be handled with. Thanks again. |
Hey, we cannot reproduce it and don't have ressources to investigate more in this topic. Do you still have the performance problem? |
Hi Eva, thank you for asking. Here are some steps to reproduce which might help you to reproduce our report:
Running this on my local dev instance with Moodle 3.6.+ and the 3.6 version of mod_organizer but with debugging = off, it took approx. 6 seconds to create the slots. Of course, this is not a real problem as creating that many slots are not an everyday task and as Moodle core has many pages which also respond that slowly. We wanted to point out that there is a performance leak and David tries to show that there are possibilities for improvement. Feel free to decide if you want to keep the report open or if you want to close it. Cheers, |
Thanks for the feedback. At the moment we don't have any resources for going deeper into this topic. |
Hi,
as we are using this plugin in our productive Moodle system, we encountered a performance issue when adding a lot of time slots (> 100 - 200).
Needing so much time slots could occur if a teacher wants to use this plugin for his consultation hours during the whole semester or if a course has more than 100 participants that all need an appointment for e.g. a presentation.
When using the organizer with only few time slots, it's responding fast (ca. one second). When dealing with a lot of slots all actions take several seconds (in my test with 224 slot always about 6 -7 seconds).
Of course the need for so much slots is surely not everyday business. Nevertheless I just wanted to tell you that the plugin does not scale to many slots considering responding times.
Cheers,
Kathrin
The text was updated successfully, but these errors were encountered: