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

grouped_parser_processor::grouped_parent_exists is called too much

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6, 2.7, 2.8, 2.9
    • Fix Version/s: 2.9
    • Component/s: Backup
    • Labels:
    • Testing Instructions:
      Hide

      1. Backup a course with users and questions
      2. Restore the course and confirm the users and question information is restored correctly.

      If you have XHProf installed and can restore the course prior to the patch, do that and save the result as a reference run.
      Then restore the course again and compare the results to ensure performance has not degraded.

      For further performance confirmation;
      1. Create a M or L course depending on your hardware.
      2. Backup that course
      3. Restore that course
      4. Apply the patch for MDL-41674
      5. Restore that course again.
      6. Compare performance of the two runs to ensure we have improved.

      Show
      1. Backup a course with users and questions 2. Restore the course and confirm the users and question information is restored correctly. If you have XHProf installed and can restore the course prior to the patch, do that and save the result as a reference run. Then restore the course again and compare the results to ensure performance has not degraded. For further performance confirmation; 1. Create a M or L course depending on your hardware. 2. Backup that course 3. Restore that course 4. Apply the patch for MDL-41674 5. Restore that course again. 6. Compare performance of the two runs to ensure we have improved.
    • Affected Branches:
      MOODLE_26_STABLE, MOODLE_27_STABLE, MOODLE_28_STABLE, MOODLE_29_STABLE
    • Fixed Branches:
      MOODLE_29_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Following on from the changes in MDL-40585, where a cache has been added to the get_parent_path function, I found from profiling that a significant time in restore is still spent on grouped_parser_processor::parent_exists.

      Adding a cache directly to this function (in addition to the existing get_parent_path cache) appears to improve performance on a large test course by about one sixth with about a 2MB memory cost.

      I did two runs before/after my code patch (alternating) using the 'M' size standard test course, with profiling turned off again. Results:

      Before: 359.5s (forgot to note memory use)
      After: 308.4s (85.6 MB)
      Before: 373.7s (83.4MB)
      After: 297.7s (85.6MB)

      Average is a 17% improvement.

      This needs review as maybe I'm missing something silly.

      If successful, it may be possible to backport this change to 2.4 and 2.5.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              mr-russ Russell Smith
              Reporter:
              quen Sam Marshall
              Peer reviewer:
              Tim Hunt
              Integrator:
              Eloy Lafuente (stronk7)
              Tester:
              Ankit Agarwal
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias
              Votes:
              2 Vote for this issue
              Watchers:
              8 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                11/May/15