Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-41728

Backup: Unnecessary use of in_array in base_plan (performance)

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.6, 2.5.2, 2.6
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: Backup
    • Labels:
    • Testing Instructions:
      Hide

      Note: This change is not supposed to result in any difference in behaviour.

      1. Go to the backup screen for a reasonably-sized test course such as the 'S' size test course from 'Make test course'.

      2. Proceed through backup using default options. When you get to the schema page, turn off a couple of activities (just to test that actually changing settings still works), then continue.

      EXPECTED: The review page should correctly indicate that you've turned off the activities.

      3. Continue through backup.

      EXPECTED: Backup completes successfully.

      4. Restore to a new course using default options.

      EXPECTED: Restore completes successfully.

      Show
      Note: This change is not supposed to result in any difference in behaviour. 1. Go to the backup screen for a reasonably-sized test course such as the 'S' size test course from 'Make test course'. 2. Proceed through backup using default options. When you get to the schema page, turn off a couple of activities (just to test that actually changing settings still works), then continue. EXPECTED: The review page should correctly indicate that you've turned off the activities. 3. Continue through backup. EXPECTED: Backup completes successfully. 4. Restore to a new course using default options. EXPECTED: Restore completes successfully.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-41728-master

      Description

      The add_task function in base_plan contains logic in a loop that does in_array on the list of settings. For very large courses, there may be thousands of settings, so this is a low-performance call.

      The setting list is already indexed by name. We can refactor the code to use the name index instead. By adding comments as well, the code will become clearer as well as slightly faster.

      Performance improvement on backup 'initial settings' screen:

      XL test course (measured using profiler):
      129.0 -> 110.5 seconds.
      14% execution time, 23% CPU time.

      L test course (no profiler, measured using perf info block, after 1 request to prime caches, 2 runs):
      15.1 -> 14.7 seconds
      15.3 -> 14.7 seconds
      3% execution time (average)

      Conclusion: This change has a small impact (half a second, 3%) on large courses, but becomes significant with very large courses where time is critical.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    11/Nov/13