Moodle
  1. Moodle
  2. MDL-40259

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      51027

      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

        Issue Links

          Activity

          Hide
          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
          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
          Martín Langhoff added a comment -

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

          Show
          Martín Langhoff added a comment - Tim's patch has my review and +1; and is in production @RL.
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24, 25 & master), thanks!
          Hide
          Petr Škoda 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
          Petr Škoda 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 Wiese added a comment -

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

          Cheers, Damyon

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

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

          Show
          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: