Moodle
  1. Moodle
  2. MDL-38161

Upgrade xhprof to latest codebase available

    Details

    • Testing Instructions:
      Hide

      Requisites:

      • To have the xhprof php extension installed and available.
      • To have the "dot" executable available.

      Settings:

      • Instead of doing that from the admin UI, put these lines in your config.php:
      $CFG->earlyprofilingenabled = true;
      $CFG->profilingenabled=true;
      $CFG->profilingautofrec = 5;
      $CFG->profilingincluded = '/*view.php,/*index.php';
      $CFG->profilingallowme = true;
      $CFG->profilingallowall = true;
      $CFG->pathtodot = '/path/to/your/executable/dot';
      

      Testing:

      1) Log into your site as admin.

      2) Reload the front page around 10-20 times (or navigate around different index.php and view.php files) and look to the footer. Randomly (the settings above define one frequency of 1/5) you will see the message "This script has been profiled".

      3) Go to various pages of moodle (course/view, participant list, backup...) and add the "PROFILEME" param to the URL (don't forget the & if needed). That will force profiling and you'll see the "This script has been profiled" always the param is used.

      4) Go to any index.php or view.php page and add the "DONTPROFILEME" to the URL (don't forget the & if needed). Reload 10-20 times and verify that the page isn't ever profiled (looking to footer).

      5) Go to config.php and delete/comment this line (because next steps do not work with early profiling enabled):

      $CFG->earlyprofilingenabled = true;
      

      6) To any URL add the "PROFILEALL" param (don't forget the & if needed). From that moment ALL the pages you visit will be profiled. All.

      7) To any URL add the "PROFILEALLSTOP". All the pages won't be profiled anymore (only the 20% random profiling tested in 2) will continue happening).

      8) Go to admin -> development -> profiling runs.

      9) There you will see a bunch of profiled scripts (from the previous tests). They are sorted by URL.

      10) Click on the date of any of them and then "View profiling details". You should get a new window/tab with the xhprof report.

      11) Verify it works by navigating, sorting, searching for some function...

      12) Don't forget to verify the graph works ([View Full Callgraph] link). This step requires "dot" to be available. By default it shows a png graph. Try adding "&type=svg" to the URL manually. If your browser supports svg, you will get the graph in that format.

      13) Go to admin -> development -> profiling runs again.

      14) Click on the "arrow" of any script having multiple profiling runs. You will get a page with the runs for that URL only.

      15) Pick some of the older runs by clicking on its date.

      16) In the page, tick the "Mark as reference run/comment" and set it's name as "base testing run". Save changes.

      17) In the breadcrumb, click the rightmost url, you will be back to the list of profiling runs for that url (same than 14) above).

      18) Verify that the run you have edited is shown in bold and with comment "base testing run".

      19) Click on any newer run. Now you should see 2 options: "View profiling details" and "View profiling differences with last reference run". Click the later.

      20) You get a new page with the summary of differences between the 2 runs, the "base" on the left, the newer on the right, with some green/red differences.

      21) Click the "View profiling diff details". You should get a new window/tab with the xhprof report showing all the diffs for each function.

      22) Verify it works by navigating, sorting, searching for some function...

      23) You can try the "View improvements/regressions callgraph", but be noted it's huge and complex to be generated and a timeout may happen. Don't fail this because of that.

      24) 2 dozens of testing points to verify it works, thanks!

      Show
      Requisites: To have the xhprof php extension installed and available. To have the "dot" executable available. Settings: Instead of doing that from the admin UI, put these lines in your config.php: $CFG->earlyprofilingenabled = true ; $CFG->profilingenabled= true ; $CFG->profilingautofrec = 5; $CFG->profilingincluded = '/*view.php,/*index.php'; $CFG->profilingallowme = true ; $CFG->profilingallowall = true ; $CFG->pathtodot = '/path/to/your/executable/dot'; Testing: 1) Log into your site as admin. 2) Reload the front page around 10-20 times (or navigate around different index.php and view.php files) and look to the footer. Randomly (the settings above define one frequency of 1/5) you will see the message "This script has been profiled". 3) Go to various pages of moodle (course/view, participant list, backup...) and add the "PROFILEME" param to the URL (don't forget the & if needed). That will force profiling and you'll see the "This script has been profiled" always the param is used. 4) Go to any index.php or view.php page and add the "DONTPROFILEME" to the URL (don't forget the & if needed). Reload 10-20 times and verify that the page isn't ever profiled (looking to footer). 5) Go to config.php and delete/comment this line (because next steps do not work with early profiling enabled): $CFG->earlyprofilingenabled = true ; 6) To any URL add the "PROFILEALL" param (don't forget the & if needed). From that moment ALL the pages you visit will be profiled. All. 7) To any URL add the "PROFILEALLSTOP". All the pages won't be profiled anymore (only the 20% random profiling tested in 2) will continue happening). 8) Go to admin -> development -> profiling runs. 9) There you will see a bunch of profiled scripts (from the previous tests). They are sorted by URL. 10) Click on the date of any of them and then "View profiling details". You should get a new window/tab with the xhprof report. 11) Verify it works by navigating, sorting, searching for some function... 12) Don't forget to verify the graph works ( [View Full Callgraph] link). This step requires "dot" to be available. By default it shows a png graph. Try adding "&type=svg" to the URL manually. If your browser supports svg, you will get the graph in that format. 13) Go to admin -> development -> profiling runs again. 14) Click on the "arrow" of any script having multiple profiling runs. You will get a page with the runs for that URL only. 15) Pick some of the older runs by clicking on its date. 16) In the page, tick the "Mark as reference run/comment" and set it's name as "base testing run". Save changes. 17) In the breadcrumb, click the rightmost url, you will be back to the list of profiling runs for that url (same than 14) above). 18) Verify that the run you have edited is shown in bold and with comment "base testing run". 19) Click on any newer run. Now you should see 2 options: "View profiling details" and "View profiling differences with last reference run". Click the later. 20) You get a new page with the summary of differences between the 2 runs, the "base" on the left, the newer on the right, with some green/red differences. 21) Click the "View profiling diff details". You should get a new window/tab with the xhprof report showing all the diffs for each function. 22) Verify it works by navigating, sorting, searching for some function... 23) You can try the "View improvements/regressions callgraph", but be noted it's huge and complex to be generated and a timeout may happen. Don't fail this because of that. 24) 2 dozens of testing points to verify it works, thanks!
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      47988

      Description

      It has been > 3 years since we incorporated xhprof profiling to moodle. Since then a bunch of commits (nothing really important) have landed upstream.

      This is about to incorporate them to moodle.

      You can find information about the (already done and tested) customizations here:

      https://github.com/stronk7/custom-xhprof/blob/MOODLE_25_STABLE/readme_moodle.txt

      And the code with the customizations applied (3 commits over stock xhprof):

      https://github.com/stronk7/custom-xhprof/compare/MOODLE_25_STABLE

      Note this does not upgrade xhprof version (0.9.2) at all so no changes in 3rdpartlibs xml is needed.

        Activity

        Hide
        Eloy Lafuente (stronk7) added a comment -

        Here you can see the candidate branch resulting of applying the the status of:

        https://github.com/stronk7/custom-xhprof/compare/MOODLE_25_STABLE

        over moodle:

        https://github.com/stronk7/moodle/compare/MDL-38161

        Basically:

        • amended (by me) readme_moodle.txt file.
        • cleanup some own (by me) comments about the integration (uppers and final dots).
        • some changes upstream about sorting files.
        • some changes upstream about supporting svg graphics.

        But nothing important nor any change required into our setup/session/db layers.

        Will create complete testing instructions and send this for integration soon.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Here you can see the candidate branch resulting of applying the the status of: https://github.com/stronk7/custom-xhprof/compare/MOODLE_25_STABLE over moodle: https://github.com/stronk7/moodle/compare/MDL-38161 Basically: amended (by me) readme_moodle.txt file. cleanup some own (by me) comments about the integration (uppers and final dots). some changes upstream about sorting files. some changes upstream about supporting svg graphics. But nothing important nor any change required into our setup/session/db layers. Will create complete testing instructions and send this for integration soon. Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Sending to integration, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Sending to integration, ciao
        Hide
        Damyon Wiese added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        Thanks!

        Show
        Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
        Hide
        Dan Poltawski added a comment - - edited

        Question for the future: should we stop bundling this with Moodle and use composer to install it? (Its a dev tool really, right?)

        Show
        Dan Poltawski added a comment - - edited Question for the future: should we stop bundling this with Moodle and use composer to install it? (Its a dev tool really, right?)
        Hide
        Eloy Lafuente (stronk7) added a comment -

        but we cannot install it from (their) upstream. It requires some minor modifications to run within Moodle:

        • basic security (require_login/cap check) to reports.
        • switch to our own "reporter" that is used intensively in the admin/xxx interface.

        Perhaps we could move it to (add-on) local or so, but I don't get the point about using the upstream version (aka composer), really. It needs hacking for sure.

        Offtopic, also, note it includes some jquery lib too (only used by their pages and not conflicting with anything, apparently). If some day jquery lands to core, that will need to be changed, surely or conflict with add-ons may happen (themes basically).

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - but we cannot install it from (their) upstream. It requires some minor modifications to run within Moodle: basic security (require_login/cap check) to reports. switch to our own "reporter" that is used intensively in the admin/xxx interface. Perhaps we could move it to (add-on) local or so, but I don't get the point about using the upstream version (aka composer), really. It needs hacking for sure. Offtopic, also, note it includes some jquery lib too (only used by their pages and not conflicting with anything, apparently). If some day jquery lands to core, that will need to be changed, surely or conflict with add-ons may happen (themes basically). Ciao
        Hide
        Dan Poltawski added a comment - - edited

        Hmm, seems weird they don't have a 'pluggable' way to do that.

        Basically I don't like that we distribute 'developer' tools with the main distribution. Perhaps composer could be a good way we could start distributing developer tools, since we're doing it already. Anyway for another discussion. (David tells me that if you wanted to you could actually 'fork' their repo and still use composer that way - to be clear thats the same as plugin.. just if one day we decided if we wanted to go that way)

        Show
        Dan Poltawski added a comment - - edited Hmm, seems weird they don't have a 'pluggable' way to do that. Basically I don't like that we distribute 'developer' tools with the main distribution. Perhaps composer could be a good way we could start distributing developer tools, since we're doing it already. Anyway for another discussion. (David tells me that if you wanted to you could actually 'fork' their repo and still use composer that way - to be clear thats the same as plugin.. just if one day we decided if we wanted to go that way)
        Hide
        Dan Poltawski added a comment -

        Integrated, thanks Eloy

        Show
        Dan Poltawski added a comment - Integrated, thanks Eloy
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Aha, I was thinking you were referring to their composer configuration (that is one of the deleted ones on our integration).

        But if we can point to our own repo with the modifications available, yes we can do that. Anyway... surely we'll need to move more stuff there like the xmldb editor, non-minified yui files... lalala all them developer tools, hehe.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Aha, I was thinking you were referring to their composer configuration (that is one of the deleted ones on our integration). But if we can point to our own repo with the modifications available, yes we can do that. Anyway... surely we'll need to move more stuff there like the xmldb editor, non-minified yui files... lalala all them developer tools, hehe. Ciao
        Hide
        Jason Fowler added a comment -

        It would seem that
        6) To any URL add the "PROFILEALL" param (don't forget the & if needed). From that moment ALL the pages you visit will be profiled. All.

        Doesn't work as intended.

        Continuing through the rest of the steps though.

        Show
        Jason Fowler added a comment - It would seem that 6) To any URL add the "PROFILEALL" param (don't forget the & if needed). From that moment ALL the pages you visit will be profiled. All. Doesn't work as intended. Continuing through the rest of the steps though.
        Hide
        Jason Fowler added a comment -

        The Profile All section doesn't work, it still only profiles the view.php and index.php pages.

        Show
        Jason Fowler added a comment - The Profile All section doesn't work, it still only profiles the view.php and index.php pages.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Uhm... strange... did you comment the $CFG->earlyprofilingenabled line (step #5) ? This is the only reason I can imagine, because PROFILEALL does not work with early profiling (because it uses sessions and sessions are not available so early).

        I've commented the line in my config.php, then added one forum discussion (/mod/forum/post.php?forum=1&PROFILEALL) and the page has been profiled. And all the subsequent pages too, until I've used the PROFILEALLSTOP.

        Cannot imagine why it's not working there.

        Show
        Eloy Lafuente (stronk7) added a comment - Uhm... strange... did you comment the $CFG->earlyprofilingenabled line (step #5) ? This is the only reason I can imagine, because PROFILEALL does not work with early profiling (because it uses sessions and sessions are not available so early). I've commented the line in my config.php, then added one forum discussion (/mod/forum/post.php?forum=1&PROFILEALL) and the page has been profiled. And all the subsequent pages too, until I've used the PROFILEALLSTOP. Cannot imagine why it's not working there.
        Hide
        Jason Fowler added a comment -

        I did comment it out, yes. I followed the instructions exactly.

        My config looks like

        //$CFG->earlyprofilingenabled = true;
        $CFG->profilingenabled=true;
        $CFG->profilingautofrec = 5;
        $CFG->profilingincluded = '/*view.php,/*index.php';
        $CFG->profilingallowme = true;
        $CFG->pathtodot = '/usr/bin/dot';
        
        Show
        Jason Fowler added a comment - I did comment it out, yes. I followed the instructions exactly. My config looks like //$CFG->earlyprofilingenabled = true ; $CFG->profilingenabled= true ; $CFG->profilingautofrec = 5; $CFG->profilingincluded = '/*view.php,/*index.php'; $CFG->profilingallowme = true ; $CFG->pathtodot = '/usr/bin/dot';
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Grrr, my fault, amending testing instructions to add:

        $CFG->profilingallowall = true;
        

        Without it PROFILEALL is not allowed, sorrrrryyyy!

        Show
        Eloy Lafuente (stronk7) added a comment - Grrr, my fault, amending testing instructions to add: $CFG->profilingallowall = true ; Without it PROFILEALL is not allowed, sorrrrryyyy!
        Hide
        Jason Fowler added a comment -

        All good now that config variable is there

        Show
        Jason Fowler added a comment - All good now that config variable is there
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Because

        A
        MARVELOUS
        A       U
        Z  YOU  P
        I  ARE  E
        N  PPL  R
        G       B
          TNKS! 
        

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Because A MARVELOUS A U Z YOU P I ARE E N PPL R G B TNKS! Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: