Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
zakkak opened this issue Sep 28, 2021 · 5 comments
Closed

Comments

@zakkak
Copy link
Collaborator

zakkak commented Sep 28, 2021

Feature request

Now that #3128 is merged it would be nice to have the option to disable the generation of the call-tree in text format when passing -H:+PrintAnalysisCallTree.

Is your feature request related to a problem? Please describe.
In complex applications the text-formatted call-tree can quickly get really big, while the CSV files are quite smaller.

E.g. for the Quarkus' integration test jpa-postgressql the text call-tree is 82G big, while the aggregated size of the CSV files is only ~17MB.

Describe the solution you'd like.
The addition of a new option to disable the generation of the call tree in text format.
An initial thought was to change PrintAnalysisCallTree from boolean to String and pass the format we want as the value, however such a solution will conflict with #3227.

As a result I am thinking about adding an extra boolean option, like PrintAnalysisCallTreeInCSV. The value of the option should be false by default, and the call tree should thus be dumped in text format only. When set to true the call tree should be dumped in csv files only.

Describe who do you think will benefit the most.
GraalVM users, GraalVM contributors, developers of libraries and frameworks which depend on GraalVM.

Express whether you'd like to help contributing this feature
I would like to contribute this once we come to an agreement on how it should be implemented.

cc @galderz

@zakkak zakkak added the feature label Sep 28, 2021
@galderz
Copy link
Contributor

galderz commented Oct 1, 2021

I'm not sure about adding another extra boolean option. How would things work if PrintAnalysisCallTreeInCSV and PrintAnalysisCallTree where set? It doesn't seem very intuitive. IMO #3227 should use a separate property for the filter, since that is complementary to either the txt or csv outputs.

@galderz
Copy link
Contributor

galderz commented Oct 1, 2021

Then you'd have PrintAnalysisCallTree being an enum. One more doubt I have is how'd that work from a backwards compatibility perspective. A lot of native image builds out there are using +PrintAnalysisCallTree so if we change that we'd have to maintain the old format as possibility to avoid breaking things. It's a popular option.

Another option would be, leave PrintAnalysisCallTree as boolean, then have a AnalysisCallTreeType enum option, with default option being text and optionally set to csv, and then have a complementary AnalysisCallTreeFilter option for implementing #3227

@zakkak
Copy link
Collaborator Author

zakkak commented Oct 4, 2021

Thanks for the feedback Galder, I really like your last proposal:

leave PrintAnalysisCallTree as boolean, then have a AnalysisCallTreeType enum option, with default option being text and optionally set to csv, and then have a complementary AnalysisCallTreeFilter option for implementing #3227

since it doesn't break backwards compatibility and seems pretty intuitive.

@christianwimmer, WDYT?

@christianwimmer
Copy link

Having PrintAnalysisCallTreeType and PrintAnalysisCallTreeFilter sound good - they can implicitly set PrintAnalysisCallTree to true when they are specified so that only one option needs to be set.

@galderz
Copy link
Contributor

galderz commented Oct 5, 2021

One more detail, when we add the PrintAnalysisCallTreeType and print either text or csv, we can remove the csv_ prefix from the .csv files. It doesn't serve any purpose.

zakkak added a commit to zakkak/mandrel that referenced this issue Oct 5, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 5, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 8, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 8, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 8, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 15, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 18, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Oct 19, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
@fniephaus fniephaus changed the title Add option to pick which kind of reports one wants to generate [GR-34638] Add option to pick which kind of reports one wants to generate Oct 20, 2021
zakkak added a commit to zakkak/mandrel that referenced this issue Nov 17, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Nov 23, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
zakkak added a commit to zakkak/mandrel that referenced this issue Nov 26, 2021
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
graalvmbot pushed a commit that referenced this issue Mar 17, 2022
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: #3843
(cherry picked from commit a134ee4)
zakkak added a commit to zakkak/mandrel that referenced this issue Mar 18, 2022
This option enables users to choose the structure of the call tree
report. Currently supported formats are txt and csv.

The use of this flag implicitly enables PrintAnalysisCallTree option.

Closes: oracle#3843
(cherry picked from commit a134ee4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants