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

Developer debug checks (for error checking) should be faster

    Details

    • Testing Instructions:
      Hide

      a. run phpunit tests
      b. install via web and CLI
      c. install with existing config.php that with/without $CFG->debug = (E_ALL | E_STRICT);
      d. upgrade from 2.2.x with/without $CFG->debug = (E_ALL | E_STRICT);


      Note: Some of the tests in this script require access to the Moodle code directory.

      1. Look at any Moodle page, for example a course view page.

      • Verify that there are no error messages or warnings caused by this feature.

      2. Rename your config.php file and point the browser at the main page for your Moodle install, so that it starts the Moodle installation process.
      3. Continue through the process as far as the point where it writes or provides config.php (or as far as you can get, if you aren't able to install due to needing to use a supposedly insecure dataroot or whatever), then stop.

      • Verify that there are no error message/warnings.

      4. Put back your normal config.php. Ensure that debugging level is set to DEBUG_ALL in your admin settings or config.php E_ALL & ~E_STRICT.
      5. Edit the course/view.php script. Near the top, before all the optional_param lines, add the following line of code:

      get_string('');
      

      6. Look at a course page.

      • No error should appear.

      7. Change debug level to DEBUG_DEVELOPER in admin UI or E_ALL | E_STRICT inconfig.php. Reload the course page.

      • An error should appear about the empty string name.

      (End of script. At this point you might want to revert the changed code file.)

      Show
      a. run phpunit tests b. install via web and CLI c. install with existing config.php that with/without $CFG->debug = (E_ALL | E_STRICT); d. upgrade from 2.2.x with/without $CFG->debug = (E_ALL | E_STRICT); Note: Some of the tests in this script require access to the Moodle code directory. 1. Look at any Moodle page, for example a course view page. Verify that there are no error messages or warnings caused by this feature. 2. Rename your config.php file and point the browser at the main page for your Moodle install, so that it starts the Moodle installation process. 3. Continue through the process as far as the point where it writes or provides config.php (or as far as you can get, if you aren't able to install due to needing to use a supposedly insecure dataroot or whatever), then stop. Verify that there are no error message/warnings. 4. Put back your normal config.php. Ensure that debugging level is set to DEBUG_ALL in your admin settings or config.php E_ALL & ~E_STRICT. 5. Edit the course/view.php script. Near the top, before all the optional_param lines, add the following line of code: get_string(''); 6. Look at a course page. No error should appear. 7. Change debug level to DEBUG_DEVELOPER in admin UI or E_ALL | E_STRICT inconfig.php. Reload the course page. An error should appear about the empty string name. (End of script. At this point you might want to revert the changed code file.)
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w33_MDL-39474_m26_debugdeveloper
    • Sprint:
      BACKEND Sprint 4

      Description

      There are two points in very frequently-used code (in the MUC cache::hash_key function, and the get_string function) where additional checks are carried out if developer debug mode is turned on.

      This is good practice but the debugging() function to check this is quite expensive.

      I propose replacing this pattern with a new global variable. This reduces the cost of checking for developer debug to the absolute minimum. (Remember, we are only 'checking if we can waste time doing extra checks' so it shouldn't take a long time to do that check, especially if it's useful in functions that may be called thousands of times in a request!)

      I've done extensive profiling of three options and believe the global variable is the best option. I'll add this in a comment.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    18/Nov/13