Extend PrintAnalysisCallTree option #3227

ziyilin · 2021-02-19T13:42:45Z

Current -H:+PrintAnalysisCallTree option prints call trees for all entries. However, the resulting file is too large to look for the method that user is interested in.

This PR extends the option to -H:PrintAnalysisCallTree=<comma-separated method list>. Only the specified entry methods' call tree will be printed out. And -H:PrintAnalysisCallTree= prints call tree for all entry methods, just like previous -H:+PrintAnalysisCallTree does.

The method name is the qualified name including parameter types, such as sun.nio.fs.UnixNativeDispatcher.<clinit>().

zakkak · 2021-04-14T14:54:29Z

Hi @mcraj017, could you please assign this to someone for a review?

munishchouhan · 2021-04-15T03:38:30Z

@zakkak thanks for the PR, I have assigned reviwers for this

zakkak · 2021-04-15T12:36:19Z

@zakkak thanks for the PR, I have assigned reviwers for this

Well, the thanks for the PR go to @ziyilin. I didn't put any effort in this, I would just like to see it merged :)
Thanks for pushing it forward.

zakkak · 2021-05-13T03:23:40Z

@christianwimmer @vjovanov are there any updates on this?

cstancu

Please document the new option format in /docs/reference-manual/native-image/Reports.md. Is this supposed to work only for entry points, or any method can be selected?

cstancu · 2021-06-22T22:54:01Z

...evm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java

+                    boolean isNative = invoke.targetMethod.wrapped.getClass().getName().equals("com.oracle.svm.jni.hosted.JNINativeCallWrapperMethod");
+                    if (print) {
+                        if (calleeNode instanceof MethodNodeReference && printedID.add(((MethodNodeReference) calleeNode).methodNode.id)) {
+                            calleeNode = ((MethodNodeReference) calleeNode).methodNode;


Why expand the method reference only for the direct invokes but not for the virtual invokes?

cstancu · 2021-06-22T22:54:53Z

...evm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java

                    if (calleeNode instanceof MethodNode) {
-                        printCallTreeNode(out, prefix + (lastInvoke ? EMPTY_INDENT : CONNECTING_INDENT) + (lastCallee ? EMPTY_INDENT : CONNECTING_INDENT), (MethodNode) calleeNode);
+                        printCallTreeNode(out, prefix + (lastInvoke ? EMPTY_INDENT : CONNECTING_INDENT) + (lastCallee ? EMPTY_INDENT : CONNECTING_INDENT), (MethodNode) calleeNode, print, null);


Passing null for the beginning parameter seems wrong.

For example testing this naively with mx helloworld -H:PrintAnalysisCallTree='java.lang.String.hashCode(),java.lang.String.toString() results in:

VM Entry Points nullentry java.lang.String.hashCode():int id=1277 nullentry java.lang.String.toString():java.lang.String id=1351

ziyilin · 2021-07-13T04:25:10Z

Please document the new option format in /docs/reference-manual/native-image/Reports.md. Is this supposed to work only for entry points, or any method can be selected?

Updated the reference file. It should report for any method selected.@cstancu please have a review. Thanks.

zakkak · 2021-09-21T11:40:42Z

Hi @ziyilin, it looks like now that #3128 got merged there are some conflicts in this PR, could you please have a look?

ziyilin · 2021-09-23T02:05:06Z

Hi @ziyilin, it looks like now that #3128 got merged there are some conflicts in this PR, could you please have a look?

@zakkak I've fixed the conflicts

galderz · 2021-10-01T12:32:16Z

Please consider my comment #3843 (comment) before integrating this. I'd rather PrintAnalysisCallTree be set to either text or csv, and then have a complementary option for the filter itself.

zakkak · 2021-10-06T07:16:04Z

docs/reference-manual/native-image/Reports.md

@@ -17,7 +17,10 @@ the image heap, respectively.
 The call tree is a a breadth-first tree reduction of the call graph as seen by the points-to analysis.
 The points-to analysis eliminates calls to methods that it determines cannot be reachable at runtime, based on the analysed receiver types.
 It also completely eliminates invocations in unreachable code blocks, e.g., blocks guarded by a type check that always fails.
-The call tree report is enabled using the `-H:+PrintAnalysisCallTree` option.
+The call tree report is enabled using the `-H:PrintAnalysisCallTree=<comma-separated method list>` option.


Hi @ziyilin , as discussed in #3843 (comment) this option would better be left as is and a new option -H:PrintAnalysisCallTreeFilter=<comma-separated method list> should be introduced instead. To avoid the need of passing both PrintAnalysisCallTreeFilter and PrintAnalysisCallTree, the former can implicitly set the latter.

I have modified as suggested.

zakkak

Thanks for the update @ziyilin .

I have added a couple of suggestion could you please have a look?

There seems to be a styling issue as well https://github.com/oracle/graal/pull/3227/checks?check_run_id=3834984765#step:9:1000

zakkak · 2021-10-08T08:55:26Z

docs/reference-manual/native-image/Reports.md

@@ -17,7 +17,11 @@ the image heap, respectively.
 The call tree is a a breadth-first tree reduction of the call graph as seen by the points-to analysis.
 The points-to analysis eliminates calls to methods that it determines cannot be reachable at runtime, based on the analysed receiver types.
 It also completely eliminates invocations in unreachable code blocks, e.g., blocks guarded by a type check that always fails.
-The call tree report is enabled using the `-H:+PrintAnalysisCallTree` option.
+The call tree report is enabled using the `-H:PrintAnalysisCallTree` option.


Suggested change

The call tree report is enabled using the `-H:PrintAnalysisCallTree` option.

The call tree report is enabled using the `-H:+PrintAnalysisCallTree` option.

zakkak · 2021-10-08T08:55:48Z

docs/reference-manual/native-image/Reports.md

-The call tree report is enabled using the `-H:+PrintAnalysisCallTree` option.
+The call tree report is enabled using the `-H:PrintAnalysisCallTree` option.
+Option `-H:PrintAnalysisCallTreeFilter=<comma-separated method list>` can be used to only print out the specified methods'
+call tree, and it implicitly enables the `-H:PrintAnalysisCallTree`.


Suggested change

call tree, and it implicitly enables the `-H:PrintAnalysisCallTree`.

call tree, and it implicitly sets `-H:+PrintAnalysisCallTree`.

zakkak · 2021-10-08T09:06:39Z

...vm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/AnalysisReporter.java

@@ -45,8 +45,12 @@ public static void printAnalysisReports(String imageName, OptionValues options,
                StatisticsPrinter.print(bb, reportsPath, ReportUtils.extractImageName(imageName));
            }

-            if (AnalysisReportsOptions.PrintAnalysisCallTree.getValue(options)) {
-                CallTreePrinter.print(bb, reportsPath, ReportUtils.extractImageName(imageName));
+            if (AnalysisReportsOptions.PrintAnalysisCallTree.hasBeenSet(options)) {


Suggested change

if (AnalysisReportsOptions.PrintAnalysisCallTree.hasBeenSet(options)) {

if (AnalysisReportsOptions.PrintAnalysisCallTree.getValue(options)) {

This condition should check whether PrintAnalysisCallTree is true, right? Not if it's just been set (possibly to false).

My mistake, I didn't change it when PrintAnalysisCallTree is changed from String to Boolean.

-H:PrintAnalysisCallTreeFilter= option accept a list of methods to print the call tree of them, the default value is an empty string to print all call trees.

zakkak · 2021-12-03T09:47:24Z

Hello @cstancu, are there any updates on this?

cstancu · 2024-06-07T14:44:53Z

This PR fell between cracks. If it's still relevant for you I have a few suggestions:

instead of using a custom filter format I would use the same method filter format as jdk.graal.MethodFilter, we used that recently also for AbortOnMethodReachable for example.
instead of using having -H:PrintAnalysisCallTreeFilter= set -H:+PrintAnalysisCallTree I would just make AnalysisCallTreeMethodFilter as a separate option, that's by default empty.

graalvmbot added the oca-signed label Feb 19, 2021

munishchouhan requested review from christianwimmer and vjovanov April 15, 2021 03:37

ziyilin force-pushed the extendPrintCallTree branch from d03afb1 to 4f44eed Compare April 15, 2021 12:44

zakkak mentioned this pull request Jun 8, 2021

Call tree analysis with graph databases (e.g. Neo4j) v2 #3128

Merged

christianwimmer requested review from cstancu and removed request for vjovanov and christianwimmer June 22, 2021 05:41

cstancu self-assigned this Jun 22, 2021

cstancu requested changes Jun 22, 2021

View reviewed changes

ziyilin force-pushed the extendPrintCallTree branch 2 times, most recently from d986983 to c73cd5a Compare July 13, 2021 04:23

ziyilin force-pushed the extendPrintCallTree branch 2 times, most recently from d1f9510 to 7b21f5c Compare September 23, 2021 02:04

zakkak mentioned this pull request Sep 28, 2021

[GR-34638] Add option to pick which kind of reports one wants to generate #3843

Closed

zakkak suggested changes Oct 6, 2021

View reviewed changes

ziyilin force-pushed the extendPrintCallTree branch from 7b21f5c to f8f93ae Compare October 8, 2021 05:01

zakkak suggested changes Oct 8, 2021

View reviewed changes

Extend PrintAnalysisCallTree option

c64d8e6

-H:PrintAnalysisCallTreeFilter= option accept a list of methods to print the call tree of them, the default value is an empty string to print all call trees.

ziyilin force-pushed the extendPrintCallTree branch from f8f93ae to c64d8e6 Compare October 8, 2021 11:55

	The call tree report is enabled using the `-H:PrintAnalysisCallTree` option.
	The call tree report is enabled using the `-H:+PrintAnalysisCallTree` option.

	call tree, and it implicitly enables the `-H:PrintAnalysisCallTree`.
	call tree, and it implicitly sets `-H:+PrintAnalysisCallTree`.

	if (AnalysisReportsOptions.PrintAnalysisCallTree.hasBeenSet(options)) {
	if (AnalysisReportsOptions.PrintAnalysisCallTree.getValue(options)) {

Extend PrintAnalysisCallTree option #3227

Are you sure you want to change the base?

Extend PrintAnalysisCallTree option #3227

Uh oh!

Conversation

ziyilin commented Feb 19, 2021

Uh oh!

zakkak commented Apr 14, 2021

Uh oh!

munishchouhan commented Apr 15, 2021

Uh oh!

zakkak commented Apr 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zakkak commented May 13, 2021

Uh oh!

cstancu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ziyilin commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zakkak commented Sep 21, 2021

Uh oh!

ziyilin commented Sep 23, 2021

Uh oh!

galderz commented Oct 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zakkak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zakkak commented Dec 3, 2021

Uh oh!

cstancu commented Jun 7, 2024

Uh oh!

Uh oh!

zakkak commented Apr 15, 2021 •

edited

Loading

ziyilin commented Jul 13, 2021 •

edited

Loading

zakkak left a comment •

edited

Loading