-
Improvement
-
Resolution: Won't Fix
-
Minor
-
None
-
2.8.9
-
MOODLE_28_STABLE
To profile yourself, add the attached performance_test.php to somewhere in Moodle. Run just it while profiling. Note: to start with, only have 3 or less items from the data provider, otherwise the profile will be far too large. After all or most of the changes from this ticket are applied, then you can increase the data provider to 20 or more to see what becomes the bottleneck when running the whole test suite.
The changes are not perfect, but they are good enough to share now. I'll discuss each commit in the following. You can see them all here: https://github.com/moodle/moodle/compare/master...moodlerooms:MDL-52255-test-perf
The problem
Our PHPUnit test suite includes over 5,600 tests and it was taking about 111 minutes to run them all. After profiling the Moodle testing framework (see above for how I did that), I discovered that after 4 database resets, the DB query count was at 5626. I then tracked down the following problems.
Problem 1: Excessive calls to moodle_database->get_columns
During database reset, several $DB calls were being executed with empty conditions (EG: delete all records from table or select all records from table). Due to another bug that I will describe next, this resulted in the database reset to query the table schema for every table in the database and then iterate over every column. The fix is simple, don't verify the columns when the conditions are empty.
See fix: https://github.com/moodle/moodle/commit/7a775e8e8071a7e0a571c3abcdb267a4fdef51d8
Problem 2: Database table reset is broken
I noticed that guess_unmodified_empty_tables was returning an empty array, which means it failed to detect if any of the table were already empty. This causes the database table reset code to reset every table in the database, which is very expensive. With the introduction of MDL-43835, the auto-increment is no longer reset to 1, but rather a random large number. But, guess_unmodified_empty_tables is still expecting to see 1 for the auto-increment of empty tables. The solution is to keep an array of expected auto-increment IDs. This fix reduces the query count by several thousands. This also made Problem 1 a lot worse than it should have been.
This fix needs some work for it to be accepted into core though:
- It only works for MySQL. Other DBs need fixing too.
- The code for MySQL when populating $lastsequenceids with defaults, is almost a straight copy/paste from reset_all_database_sequences - so should probably be moved into a function.
See fix: https://github.com/moodle/moodle/commit/75b80db1b82d667a1df74026a40e0ffba58740f5
Problem 3: Database table reset is still expensive
Even with Problem 2 fixed, the table reset for tables that have records in them after the initial install (EG: course, config, user, etc) is still expensive. This is because you have to query all rows and do some inspecting. I think it does this for 26'ish tables for every database reset? So, the fix for this is to track all of the modified tables and only reset those. This fix is for PHPUnit only.
See fix: https://github.com/moodle/moodle/commit/5a47cebe7928ceffd097acf5725c228f58afb9ee
Problem 4: Improve get_message_processors reset
This is a very minor improvement, but saves 2 or more queries for every test reset.
See fix: https://github.com/moodle/moodle/commit/2795d8c2267a7d378a114e67fdea070b82c91d0f
Results
So after all that, the performance test shows that the query count went from 5626 to around 70 for 4 database resets. Our test execution time went down from 111 minutes to around 45 minutes for the whole PHPUnit test suite. In addition, I have only mentioned PHPUnit, but fixes for Problems 1, 2 and 4 also help Behat, so bonus!
There is still room for improvement
- moodle_database->get_columns is called very often and the results are purged after every test. I wrote some crappy code to try to cache this and it did improve whole test suite execution time by another 5-7 minutes. But, the code isn't great and it did break other tests.
- Might be a good idea to somehow aggregate some stats, like total number of DB queries, to help prevent things like this in the future.
- This looks cool: https://github.com/johnkary/phpunit-speedtrap - I might try this out, but integration into core would make use easier.
- I feel like the file cache is slowing things down. I was playing around here and I think I found that using the dummy cache was actually faster. This was before the above problems were fixed, so might be a non-issue now.