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

Memory used and files included increase detected in all course view requests

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      1) Enable $CFG->enableasyncbackup

      2) Before patch, measure memory usage of course view page, with warm caches eg load a couple times. Do this for view page and with settings on

      3) Now apply patch.

      4) Re test memory when viewing the page with editing off, memory should be lower

      5) Turn editing on, memory should be higher pulling in the extra libs

      6) Disable $CFG->enableasyncbackup

      7) Re test course with editing still on, memory should be lower with libs not loaded

       

       

       

       

       

      Show
      1) Enable $CFG->enableasyncbackup 2) Before patch, measure memory usage of course view page, with warm caches eg load a couple times. Do this for view page and with settings on 3) Now apply patch. 4) Re test memory when viewing the page with editing off, memory should be lower 5) Turn editing on, memory should be higher pulling in the extra libs 6) Disable $CFG->enableasyncbackup 7) Re test course with editing still on, memory should be lower with libs not loaded          
    • Affected Branches:
      MOODLE_37_STABLE
    • Fixed Branches:
      MOODLE_37_STABLE
    • Pull Master Branch:
      MDL-65321-course-perf

      Description

      This is a regression introduced by MDL-28505 (master only) and should be fixed before 3.7 release (aka, must fix).

      Our performance comparison scripts detected a 6% memory increase in all course view requests. Problem was traced down to the new async backups option where, irrespectively of them being enabled or no (site-wide setting), the increase happens.

      In Rex Lorenzo wise words:

      I see the increased in files being included is because of the notification for a pending async here:
      https://github.com/moodle/moodle/compare/master...mattporritt:master_MDL-28505_Asynchronous_backup_and_restore#diff-74f014905906a752b55944bb01490677R8

      So, we should not be including all the backup machinery unconditionally, the check should be really lighter, specially when the feature is disabled (also when enabled).

      That's it, ciao

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                brendanheywood Brendan Heywood
                Reporter:
                stronk7 Eloy Lafuente (stronk7)
                Peer reviewer:
                Matt Porritt
                Integrator:
                Eloy Lafuente (stronk7)
                Tester:
                Eloy Lafuente (stronk7)
                Participants:
                Component watchers:
                Adrian Greeve, Mihail Geshoski, Peter Dias, Matteo Scaramuccia, Jake Dallimore, Jun Pataleta, Ryan Wyllie
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  20/May/19

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 50 minutes
                  50m