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

Upgrade xhprof to latest codebase available

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Component/s: General, Libraries, Performance
    • Labels:
      None
    • 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:

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Sending to integration, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Sending to integration, ciao
            Hide
            damyon 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 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
            poltawski 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
            poltawski 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
            stronk7 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
            stronk7 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
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            Integrated, thanks Eloy

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks Eloy
            Hide
            stronk7 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
            stronk7 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
            phalacee 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
            phalacee 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
            phalacee Jason Fowler added a comment -

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

            Show
            phalacee Jason Fowler added a comment - The Profile All section doesn't work, it still only profiles the view.php and index.php pages.
            Hide
            stronk7 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
            stronk7 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
            phalacee 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
            phalacee 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
            stronk7 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
            stronk7 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
            phalacee Jason Fowler added a comment -

            All good now that config variable is there

            Show
            phalacee Jason Fowler added a comment - All good now that config variable is there
            Hide
            stronk7 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
            stronk7 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
            Hide
            tsala Helen Foster added a comment -

            A QA test now exists for this issue - MDLQA-7164 - so I am removing the qa_test_required label. You'll notice I have just copied the testing instructions from this issue into the QA test. Please shout if it needs amending, thanks!

            Show
            tsala Helen Foster added a comment - A QA test now exists for this issue - MDLQA-7164 - so I am removing the qa_test_required label. You'll notice I have just copied the testing instructions from this issue into the QA test. Please shout if it needs amending, thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Super Helen, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Super Helen, thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/May/13