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

Backup: Unnecessary use of in_array in base_plan (performance)

    XMLWordPrintable

    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.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              quen Sam Marshall
              Reporter:
              quen Sam Marshall
              Peer reviewer:
              Tim Hunt
              Integrator:
              Eloy Lafuente (stronk7)
              Tester:
              Jason Fowler
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

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