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

assignsubmission_comments: Improve performance for permissions checking

XMLWordPrintable

    • MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • MOODLE_26_STABLE, MOODLE_27_STABLE
    • MDL-45678-006-master
    • 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.

      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.

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

            netspot_dbezborodov Damien Bezborodov
            netspot_dbezborodov Damien Bezborodov
            Damyon Wiese Damyon Wiese
            Sam Hemelryk Sam Hemelryk
            Ankit Agarwal Ankit Agarwal
            Votes:
            2 Vote for this issue
            Watchers:
            10 Start watching this issue

              Created:
              Updated:
              Resolved:

                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.