Callgraph support

It's a little hard to view as a diff given that it doesn't look like you retained the history on github, but if you can point me to a repo that I can add as a remote and diff/merge from, I'm happy to give it a second look.

This isn't a feature that I ever use, but I'm fine with bringing it if you want to finish it up.

Repo worked a treat for me, thanks!

This also works great for me right out of the box. Thanks a lot.

this would be real nice to have merged & commited. arthurf is it possible to create a patch that can be commited? that's much easier for the maintainers, generally, as it seems they dont have a lot of time to spend on this module anyway.

Yes please for getting a patch and having this committed to the main project... would hate to lose this work down the road.

This is a pretty old case but it would be really nice if it could be merged into the current module code.

Here's a patch based on the above to add Callgraph support to the current 7.x-1.0-beta3 version.

The changes to the existing module files are pretty basic: The link to view the callgraph, the menu callback and addition of the callback function. There is also the xhprof_lib rolled into it as well which has minor changes to get it to work.

This probably [almost definitely] should be rewritten to use the library files that are included with the module.

At best, this should be marked 'Needs review', since we finally have a patch (thanks, oliverpolden!).

Since Devel doesn't include XHProf integration (which I used to use with GraphViz and an Apache virtualhost to see callgraphs), this seems like it would be the last missing piece to make this module a complete integration of the basic components of XHProf with Drupal :)

updated oliverpolden's patch to use "a/" and "b/" directories instead of "sites/all/modules/xhprof/"

Thx for this patch, this feature was missing. Works as expected.

I don't know why @nbucknor added a/ b/ directories in #10.

IMHO the proper directory for the callgraph library should be:
/sites/all/modules/xhprof/XHProfLib/callgraph or something similar.

The patch from #8 also creates an incorrect directory (probably due to case sensitivity).
BTW, thanks for the original patch. This is a valuable addition to the xhprof module.

Here's another patch against 1.x-dev to correct the callgraph library directory.

This node was unpublished in error. My apologies. It is now republished.

Confirmed patch #12 worked out of the box.

So many of us are still doing D7 jobs! This should be committed!

It works, so could be committed probably.
But I'd like to hear from the maintainers what they want to do with the 3rd party library.
Do the maintainers want the library file within a module subdir or just included by users themselves in their sites/*/libraries dir?
Or maybe as a remote resource with a separate .make file perhaps?

@basvredeling I switched to a/ b/ directories because I thought it was best practice to set a patch in the project root and not the docroot. I my case, contributed modules are stored in sites/all/modules/contrib not sites/all/modules.

@nbucknor thanks for the reply. It's common practice to run patches against the project root. No matter where you place the module itself, the patch should always apply cleanly. So the neither "sites/all/modules/xhprof" nor "a/ b/" dir structures should probably end up in the patch.

Revisiting this issue, I think it would be better to include the xhprof library as a separate library using the libraries module. Make the dependency on libraries optional and include a readme and .make file to instruct users on placing this optional lib.

I tried #12 against 7.x-1.x-dev but when clicking the "call graph" link /admin/reports/xhprof/callgraph/5b4f066f85c3a on the report nothing gets displayed

instead "Symbol 5b4f066f85c3a not found in XHProf run"

Run Report
Run #callgraph: Invalid Run Id = callgraph

The report itself /admin/reports/xhprof/5b4f066f85c3a works.

Please help :)

I tried installing it again and flushed the cache, now when I click "call graph" it renders an empty page :(

<body style="margin: 0px; background: #0e0e0e;" data-gr-c-s-loaded="true"><img style="-webkit-user-select: none;" src="https://drupalserver/admin/reports/xhprof/callgraph/5b4f066f85c3a"></body>

#12 would not apply to the latest 7.x-1.x-dev, so here's a simple reroll.

Here is an updated version for D8 branch.

Note that if you use this patch in cunjunction with #2354853: The path is not shown in the overview of xhprof logs. you will have to also change the following :

diff --git a/src/XHProfLib/Callgraph/utils/xhprof_runs.php b/src/XHProfLib/Callgraph/utils/xhprof_runs.php
index 5ee8ca9..24b758c 100644
--- a/src/XHProfLib/Callgraph/utils/xhprof_runs.php
+++ b/src/XHProfLib/Callgraph/utils/xhprof_runs.php
@@ -118,7 +118,7 @@ class XHProfRuns_Default implements iXHProfRuns {
 
     $contents = file_get_contents($file_name);
     $run_desc = "XHProf Run (Namespace=$type)";
-    return unserialize($contents);
+    return unserialize($contents)['data'];
   }
 
   public function save_run($xhprof_data, $type, $run_id = null) {

Unfortunately, the patch in #21 is not working for me.

Whenever I click the "graph" link, I get the following error:

The website encountered an unexpected error. Please try again later.
DivisionByZeroError: Division by zero in xhprof_generate_dot_script() (line 368 of modules/contrib/xhprof/src/XHProfLib/Callgraph/utils/callgraph_utils.php).
xhprof_generate_dot_script(Array, 0.01, 'TEST', 'XHProf Run (Namespace=TEST)', '', 1) (Line: 450)
xhprof_get_content_by_run(Object, '625024af0fea3', 'svg', 0.01, '', 'TEST', 1) (Line: 478)
xhprof_render_image(Object, '625024af0fea3', 'svg', 0.01, '', 'TEST', 1) (Line: 194)
Drupal\xhprof\Controller\XHProfController->graphAction(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 564)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 49)
Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Test environment:
PHP 8.0
Drupal 9.3.9
tideways_xhprof installed on a platform.sh lando local dev environment

Did you add also the little change described at #21 for the other related patch ? Both works in cunjunction.

@Dom. thanks for the quick response. I did not have the other patch installed; I only installed this one.

So, I then installed the other patch and made the change described in #21, but I still have the same "division by zero" error.

@ptmkenny :
I confirmed I just tested at the moment on a real up-to-date real :
- drupal 3.9.5
- PHP 8.1.1
- XHProf 2.3.5 installed via pecl install xhprof-2.3.5
- graphviz 2.44.0 installed via apk add graphviz
- this module installed using GIT :

git clone https://git.drupalcode.org/project/XHProf.git
cd xhprof
git apply add_graph--1470740--21.patch

Tested with all default options from the module, as well as enabled all three options :
CPU
Memory
Exclude PHP builtins function

In both case, I could get my graph with no issues at all.

Can you be more specific about the tideways_xhprof version ?

Also, you can fully comment the incriminated lines by adding double slashes // in front if lines 365 to 369 so it looks as such :

//      $taillabel = ($sym_table[$parent]["wt"] > 0) ?
//        sprintf("%.1f%%",
//                100 * $info["wt"] /
//                ($sym_table[$parent]["wt"] - $sym_table["$parent"]["excl_wt"]))
//        : "0.0%";

Those are the percentage noted after the function boxes :

Thanks @dom.

Using tideways/php-xhprof-extension 5.0.4.

The issue was that I didn't have graphviz installed. It might be nice if there was a way to inform the user that graphviz needs to be installed instead of a divide by zero error.

Got this error on visiting graph page:
failed to execute cmd: " dot -Tsvg". stderr: `sh: 1: dot: not found '

FYI xhprof using dot too, but last versions changed default to use svg render because it scalable and has no issues with fonts (PNG requires huge package of fonts) to work

Ref https://github.com/longxinH/xhprof/pull/57

Patch #21 is working for me. I'm using PHP 8.1 and xhprof 2.3.5 installed via pecl.

Patch #21 worked for me as well! Thanks, everyone! 🙌🏼
(Update after #27)

How I fixed my error mentioned in #27? Well, I was using lando setup. So had to install the required package and extensions within lando ssh.

Rebased the patch on 2.x.

@mortana2k, what are you talking about rebased the patch on 2.x ? The Merge request !7 is against 8.x-1.x

We should have a new merge request for 2.x since Merge request !7 is against 8.x-1.x

I applied the patch to the 2.x branch and pushed it as MR7. But looks like I forgot to set the MR to merge into 2.x. Just did that.

ok great, that looks a lot better, thanks!

I can confirm that the changes in MR 7 work well and produce graphs. Thank you!

The only thing to watch out for is that if graphviz isn't installed it throws a very odd looking error on a white screen of death.

failed to execute cmd: " dot -Tsvg". stderr: `sh: 1: dot: not found '

If we can't detect the error in code then it just needs to be stated explicitly in the readme file.