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.

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

Open
Kathrin84 opened this issue Nov 4, 2015 · 13 comments
Open

Performance Issue #11

Kathrin84 opened this issue Nov 4, 2015 · 13 comments
Labels

Comments

@Kathrin84
Copy link

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

@mkemmerling
Copy link
Contributor

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,
Markus Kemmerling

@kaspot
Copy link
Member

kaspot commented Jun 1, 2016

H i Kathrin,
the developer checked this issue and tested the creating of a bigger amount of slots and hasn’t had any performance issues. He also reported that he doesn’t see any reason why the performance should be bad — especially when it’s only writing data entries into a table.
May I ask you to give us more specific data if you want us to keep digging.

Thanks.

@kaspot
Copy link
Member

kaspot commented Jul 25, 2016

Hi Kathrin,
is this issue still occurring on your instance? If yes, may I ask you for more detailed information so that we can start looking into it in more detail.
Thank you.

@kaspot
Copy link
Member

kaspot commented Sep 20, 2016

no further information since last year, therefor we are closing the issue.

@kaspot kaspot closed this as completed Sep 20, 2016
@Kathrin84
Copy link
Author

Kathrin84 commented Sep 21, 2016

Hi,

I did some further investigation at the beginning of august and then I was so busy with other projects and ill so that I had no time to give you some further information.

Although the ticket is closed yet, I will post it here so that my research was not in vain.

I created 2 slots and the other time quite a bunch of slots (7386).

In Summary it seems that there are too many database queries.
By adding 2 slots it were 155.
By adding 7386 slots it were 51602 database queries.

Even deleting slots took several seconds.

I attach my screenshots, so you can have a look over them by yourself:
2slots_analyse
2slots_delete
2slots_xhprof
7386slots_xhprof_2
7386slots_xhprof_analyse
7386slots_xhprof

Cheers, Kathrin

@abias
Copy link

abias commented Sep 21, 2016

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,
Alex

@kaspot
Copy link
Member

kaspot commented Sep 21, 2016

Hi Kathrin, Hi Alex,
a big thanks to both of you for the very detailed research and the improvement ideas. I will forward this to our developer and hope he can add it to his 3.2 release plan.

@abias
Copy link

abias commented Sep 22, 2016

Hi Kasia,

you're welcome.
Would you mind re-opening the ticket here in Github then if you plan to continue to investigate this topic?

Thanks,
Alex

@kaspot kaspot reopened this Sep 22, 2016
@davidknu
Copy link

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.
By using two bulk inserts (one select) and one (bulk) update (to connect the ids), I was able to speed up this part by factor 20 (I was testing with aprox. 100, 5000 and 15000 slots).

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);
$result = $DB->get_records('organizer_slots', $conditions);

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";
$updatequery = "UPDATE {organizer_slots} SET eventid=(".$subquery.") WHERE organizerid = ".$organizer->id." AND eventid IS NULL";

(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,
David

@kaspot
Copy link
Member

kaspot commented Jan 9, 2018

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.

@goggo24
Copy link
Member

goggo24 commented Apr 15, 2019

Hey, we cannot reproduce it and don't have ressources to investigate more in this topic. Do you still have the performance problem?

@abias
Copy link

abias commented May 2, 2019

Hi Eva,

thank you for asking.

Here are some steps to reproduce which might help you to reproduce our report:

  • Login as teacher
  • Go to a course
  • Create an organizer activity with default settings
  • Add new slots
    ** Teacher: The teacher user
    ** Location: Foo
    ** Duration: 15 Minutes
    ** Gap: 0
    ** Participants: 1
    ** Start date: 15th april 2020
    ** End date: 15th august 2020
    ** Weekday slot: Monday from 14:00 to 18:00
    ** (This is a realistic setup for consultation hours for a whole lecture term and will result in 272 slots for 288 persons)

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,
Alex

@goggo24
Copy link
Member

goggo24 commented May 17, 2019

Thanks for the feedback. At the moment we don't have any resources for going deeper into this topic.
I'll leave the issue open, maybe we will find time in the future for working on it. Cheers, Eva

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants