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

mod/scorm: significant memory improvements on large datasets with get_users_by_capability fix

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.10, 2.3.7, 2.4, 2.5, 2.6
    • Fix Version/s: 2.3.8, 2.4.5, 2.5.1
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      View the reports page of a SCORM package that has some attempts from students.
      View both the Basic and interactions reports and make sure they display the users correctly - check the exports of the report to make sure those are generated correctly as well.

      Show
      View the reports page of a SCORM package that has some attempts from students. View both the Basic and interactions reports and make sure they display the users correctly - check the exports of the report to make sure those are generated correctly as well.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      get_users_by_capability_fix_master

      Description

      In Moodle we have a database of about 40,000 users and our basic and interactions scorm reports span hundreds of pages.

      We've encountered PHP memory exhaustion errors that we've traced to the get_users_by_capability calls in mod/scorm/report/<basic|interactions>/report.php

      In order to fix the memory errors we had to set the PHP memory_limit > 512M from the default of 128M.

      Our fix involves limiting the fields returned from get_users_by_capability to user ID and unsetting variables to decrease memory utilization. These are the performance numbers (with APC cache enabled) when running the scorm reports before and after the applied patch:
      Pre-patch memory usage (peak MB): 616.4
      Post-patch memory usage (peak MB): 97.1

      After applying the patch we can use the PHP default memory limit of 128M.

      Please take a look at the the following branches at http://github.com/timgus/moodle
      get_users_by_capability_fix_m22
      get_users_by_capability_fix_m23
      get_users_by_capability_fix_m24
      get_users_by_capability_fix_m25
      get_users_by_capability_fix_master

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            danmarsden Dan Marsden added a comment -

            yeah - get_users_by_cap is pretty heavy - looks like a simple fix to make with a good improvement - adding my +1 for integration but I'm unlikely to get to this for a couple of weeks as I'll be away at the Melbourne moot - happy for anyone else to pick this up and submit it to supported stable releases - thanks for the fix!

            Show
            danmarsden Dan Marsden added a comment - yeah - get_users_by_cap is pretty heavy - looks like a simple fix to make with a good improvement - adding my +1 for integration but I'm unlikely to get to this for a couple of weeks as I'll be away at the Melbourne moot - happy for anyone else to pick this up and submit it to supported stable releases - thanks for the fix!
            Hide
            martinlanghoff Martín Langhoff added a comment -

            Tim's patch has my review and +1; and is in production @RL.

            Show
            martinlanghoff Martín Langhoff added a comment - Tim's patch has my review and +1; and is in production @RL.
            Hide
            danmarsden Dan Marsden added a comment -

            Thanks Martin - bouncing this up for integration but removing 2.2 branch from this as we no longer support it for bug fixes (only just making it into 2.3 as we drop bug fix support for 2.3 this month too.

            Show
            danmarsden Dan Marsden added a comment - Thanks Martin - bouncing this up for integration but removing 2.2 branch from this as we no longer support it for bug fixes (only just making it into 2.3 as we drop bug fix support for 2.3 this month too.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (23, 24, 25 & master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23, 24, 25 & master), thanks!
            Hide
            skodak Petr Skoda added a comment -

            1/ master is fine
            2/ 2.5.x - Notice: Undefined property: stdClass::$toc in /Users/skodak/server/workspace/moodle25/mod/scorm/locallib.php on line 1598
            3/ 2.4.x - Notice: Undefined property: stdClass::$toc in /Users/skodak/server/workspace/moodle24/mod/scorm/locallib.php on line 1598
            4/ 2.3.x - Notice: Undefined property: stdClass::$toc in /Users/skodak/server/workspace/moodle23/mod/scorm/locallib.php on line 1302

            So looking at the patch the $toc notices are probably not related, so passing.

            Please deal with the notices somehow yourself, thanks.

            Show
            skodak Petr Skoda added a comment - 1/ master is fine 2/ 2.5.x - Notice: Undefined property: stdClass::$toc in /Users/skodak/server/workspace/moodle25/mod/scorm/locallib.php on line 1598 3/ 2.4.x - Notice: Undefined property: stdClass::$toc in /Users/skodak/server/workspace/moodle24/mod/scorm/locallib.php on line 1598 4/ 2.3.x - Notice: Undefined property: stdClass::$toc in /Users/skodak/server/workspace/moodle23/mod/scorm/locallib.php on line 1302 So looking at the patch the $toc notices are probably not related, so passing. Please deal with the notices somehow yourself, thanks.
            Hide
            damyon Damyon Wiese added a comment -

            This issue is fixed! Hurray! Hurray!
            Your issue is fixed, what a wonderful day!

            Cheers, Damyon

            Show
            damyon Damyon Wiese added a comment - This issue is fixed! Hurray! Hurray! Your issue is fixed, what a wonderful day! Cheers, Damyon
            Hide
            danmarsden Dan Marsden added a comment -

            thanks Petr -created MDL-40512 for the undefined warnings.

            Show
            danmarsden Dan Marsden added a comment - thanks Petr -created MDL-40512 for the undefined warnings.

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13