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

assignsubmission_comments: Improve performance for permissions checking

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      1) Open a assignment Grading Table with over 50 rows with submissions and comments per page.

      2) Verify web server and database performance metrics and page load time.

      3) Apply the fix.

      4) Verify performance improvement for the grading table.

      5)
      5) a) Suspend a student (Suspend the enrolment not the student account)
      5) b) Log in as a tutor without capability 'moodle/course:viewsuspendedusers'
      5) c) Should not be able to view the suspended user in the assignment grading table.

      Show
      1) Open a assignment Grading Table with over 50 rows with submissions and comments per page. 2) Verify web server and database performance metrics and page load time. 3) Apply the fix. 4) Verify performance improvement for the grading table. 5) 5) a) Suspend a student (Suspend the enrolment not the student account) 5) b) Log in as a tutor without capability 'moodle/course:viewsuspendedusers' 5) c) Should not be able to view the suspended user in the assignment grading table.
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE, MOODLE_27_STABLE
    • Pull Master Branch:
      MDL-45678-006-master

      Description

      When viewing the grading table, a permissions callback will check for suspended users for each submission in the table.

      Please view the attached callgraph alongside my explanation regarding my changes.

      In assign_submission_comments::view_summary(), there is a call to the comment constructor. This constructor calls comment::check_permissions(), which is expensive – for each row!

      According to the callgraph, the reason why comment::check_permissions() is expensive, is not because it calls has_capability(), but rather because it calls assignsubmission_comments_comment_permissions().

      If you view assignsubmission_comments_comment_permissions(), it checks get_suspended_userids(), which generates an expensive SQL query in pg_query_params().

      My fix is to cache the result of get_suspended_userids() in a MODE_REQUEST Cache.

        Attachments

        1. grading-screen-master27.png
          grading-screen-master27.png
          170 kB
        2. grading-screen.png
          grading-screen.png
          213 kB
        3. callgrind.out
          566 kB
        4. callgraph.svg
          231 kB
        5. callgraph.png
          callgraph.png
          4.38 MB

          Issue Links

            Activity

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                10 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/Jul/14