Maoni0 / realmon

A monitoring tool that tells you when GCs happen in a process and some characteristics about these GCs
MIT License
281 stars 25 forks source link

Added the New Configuration Manager invoked through a command line arg that allows the user to create new configurations based on a prompt #19

Closed MokoSan closed 2 years ago

MokoSan commented 2 years ago

By passing in -g <Path to config>, the user is able is create and persist a new config file based on a prompt such as the following:

C__Users_mukun_source_repos_realmon_src_GCRealTimeMon_bin_Debug_net6 0_GCRealTimeMon exe 2021-11-19 22-21-57

Based on the choices of the prompt and the path provided in the command line, a new config file will be persisted and can be used in subsequent runs. The choice of command line args are:

If neither -g nor -c is specified, deserialize: DefaultConfig.yaml. If -c is specified without an argument, open up the prompt, overwrite DefaultConfig.yaml and make use of the the new configs in the session. <- this was a bit hacky considering the CommandLineParser Options are strongly typed i.e. I couldn't specify both a bool and a string for the path as the type of that argument. If -c is specified with an argument, deserialize the path specified as the argument. If -g is specified, open up the prompt, persist the new settings in the path specified and make use of the new configs in the session.

chrisnas commented 2 years ago

By passing in -g <Path to config>, the user is able is create and persist a new config file based on a prompt such as the following:

C__Users_mukun_source_repos_realmon_src_GCRealTimeMon_bin_Debug_net6 0_GCRealTimeMon exe 2021-11-19 22-21-57

Based on the choices of the prompt and the path provided in the command line, a new config file will be persisted and can be used in subsequent runs.

This is a great idea to allow the user to select the columns in the console! However, why not simply using -c without parameter to indicate that columns will be selected and saved in the default .yaml file? That way, not need to deal with new config file :^)

In addition, you should rebase with the current state of the code because it has been refactored a lot; especially for parsing parameters and handle the session in a more consistent way.

Last but not least, don't forget to update the version in the dotnet-gcmon.csproj file

MokoSan commented 2 years ago

This is a great idea to allow the user to select the columns in the console! However, why not simply using -c without parameter to indicate that columns will be selected and saved in the default .yaml file? That way, not need to deal with new config file :^)

Thanks for the great idea!! I have included the following behavior:

  1. If neither -g nor -c is specified, deserialize: DefaultConfig.yaml.
  2. If -c is specified without an argument, open up the prompt, overwrite DefaultConfig.yaml and make use of the the new configs in the session. <- this was a bit hacky considering the CommandLineParser Options are strongly typed i.e. I couldn't specify both a bool and a string for the path as the type of that argument. Either way, happy I got this working.
  3. If -c is specified with an argument, deserialize the path specified as the argument.
  4. If -g is specified, open up the prompt, persist the new settings in the path specified and make use of the new configs in the session.

In addition, you should rebase with the current state of the code because it has been refactored a lot; especially for parsing parameters and handle the session in a more consistent way.

Done! Totally forgot to rebase but now have fixed all merge conflicts. Thanks for catching this!

Last but not least, don't forget to update the version in the dotnet-gcmon.csproj file Done!

Thank you for all the great feedback, @chrisnas! Please keep it coming if you think anything else can be improved.

Maoni0 commented 2 years ago

thank you both!! :)

if I think about how I would use a new tool, I'd probably start by typing "dotnet gcmon /?" or simply "dotnet gcmon" just to see what it tells me. right now it says Specify a process Id using: -p or a process name by using -n.. what if we displayed the help when the user doesn't specify an arg or specifies "/?"? like this

Usage -
Specify a process Id by using -p or a process name by using -n, the tool will show GCs as they occur in that process. If there are multiple processes with that name it would pick the first one
Specify a min GC pause in ms by using "-m x" to only display GCs with pauses above x ms
You can specify which info to display per GC by using a config file. You can either change the current config at [fill in the path to DefaultConfig.yaml here] or specify your own by using "-c config_file_path", eg, "-c c:\data\gcmon-config.yaml" 
To create a config file, use -g which allows you to specify a path for the config file and choose which info to display for each GC

does this seem like it would make starting using this tool easier? (please feel free to make the text more clearl)

also just a side thought, since we have a config file, we should probably deprecate the "-m" option and make that part of the config?

MokoSan commented 2 years ago

@Maoni0, I think your incorporated feedback definitely improves the overall user experience. :)

I have added the following help text in case the user:

The final help text looks like the following: image

I have also removed the -m command line arg and replaced it with specifying the following in the config:

display_conditions:
    min gc duration (msec): 200

Here is a gif of the new prompt menu:

C__Users_mukun_source_repos_realmon_src_GCRealTimeMon_bin_Debug_net6 0_GCRealTimeMon exe 2021-11-20 22-04-52

Maoni0 commented 2 years ago

thanks! I just tried this out. BTW I should clarify what I had here in the help text -

You can specify which info to display per GC by using a config file. You can either change the current config at [fill in the path to DefaultConfig.yaml here] or specify your own by using "-c config_file_path", eg, "-c c:\data\gcmon-config.yaml"

I meant [fill in the path to DefaultConfig.yaml here] is to be replaced by the actual path of DefaultConfig.yaml on the machine :)

To create a config file, use -g which allows you to specify a path for the config file and choose which info to display for each GC or overwrite the default config by entering -c without any parameters.

I tried GCRealTimeMon.exe -c and GCRealTimeMon.exe -g c:\temp\myconfig.yamlbut it seems to tell me the same help text... from the text it sounds like GCRealTimeMon.exe -c should also bring up the part that allows me to select which info to display except it should be pre-populated with what's in the default?

MokoSan commented 2 years ago

I meant [fill in the path to DefaultConfig.yaml here] is to be replaced by the actual path of DefaultConfig.yaml on the machine :)

Fixed.. lol

I tried GCRealTimeMon.exe -c and GCRealTimeMon.exe -g c:\temp\myconfig.yamlbut it seems to tell me the same help text... from the text it sounds like GCRealTimeMon.exe -c should also bring up the part that allows me to select which info to display except it should be pre-populated with what's in the default?

I should have updated this logic.. What I had before was that we absolutely needed the pid / process name for anything to happen. With the new logic, if the user passes -c or -g <path>, they will automatically go to the configuration creation mode.

Now, for the -c command line arg, I am reading the selected values e.g. columns from the default. For example, for the following config:

columns:
- type
- pause (ms)
- gen0 alloc (mb)
stats_mode:
  timer: 20s
display_conditions:
  min gc duration (msec): 200

The prompts will be: C__Users_mukun_source_repos_realmon_src_GCRealTimeMon_bin_Debug_net6 0_GCRealTimeMon exe 2021-11-20 23-51-31

For the sake of completeness, here are the main actions as per the user specified command line args:

Command Line Action
No Args Help displayed
-? or --/? Help Displayed
-c Prompt to generate a config file to overwrite the DefaultConfig.yaml with defaults from that file itself.
-g (path) Prompt to generate a config file in the desired path.
-c -n (processName) Same as -c but this time, realmon will also start the monitoring.
-g (path) -n (processName) Same as -g (path) but this time, realmon will also start the monitoring.
-n (processName) Configs are read from DefaultConfig.yaml and realmon will start the monitoring.
chrisnas commented 2 years ago

For the sake of completeness, here are the main actions as per the user specified command line args:

Command Line Action No Args Help displayed -? or --/? Help Displayed -c Prompt to generate a config file to overwrite the DefaultConfig.yaml with defaults from that file itself. -g (path) Prompt to generate a config file in the desired path. -c -n (processName) Same as -c but this time, realmon will also start the monitoring. -g (path) -n (processName) Same as -g (path) but this time, realmon will also start the monitoring. -n (processName) Configs are read from DefaultConfig.yaml and realmon will start the monitoring.

Here is what you get for other dotnet CLI tools such as dotnet counters which is close to what gcmon is doing: image

So help is displayed like you implemented (no parameter, -?) but also with -h and --help. If a command is given without mandatory options, the corresponding help is provided. And -h for a given command details the options for that specific command: image

Also, each command is exclusive and options apply to commands. In our case, we could have the following commands: monitor -p <pid> -c <input yaml file - if not provided, enter your great column selection mode> configure <output yaml file - if not provided, enter your great column selection mode and save into default yaml>

Also, we could imagine supporting the same collect command that would save the result in a .csv file. Since I'm thinking about the ways to use dotnet gcmon in scripts, the -d(uration) option for collect command could be interesting so it would run for the given duration and save the output into the given file: dotnet gcmon collect -p <pid> -n <name> -o(utput) <file name> -f(ormat) <csv|json - csv is the default if not provided> I'm willing to implement that new collect command in another PR so you don't add unrelated code to your PR if you prefer :^)

MokoSan commented 2 years ago

Great suggestions - will clean up the help text unless there is a way to automatically do it. The collect command seems like a great addition.

MokoSan commented 2 years ago

@chrisnas @Maoni0:

Below what the new usage text looks like, please let me know if any other changes are needed. This shows up iff:

image

Maoni0 commented 2 years ago

when I answered 'n' to

Would you like to set a value for the minimum GC Pause duration to filter GCs off of?

it just shows me the help text again

image

also the selected columns are very difficult to read being in one very long line.. would be great to keep each column as its own line.

I'm unclear what this means -

  -c, --configPath          The path to the YAML configuration file that is read
                            in. If no path is specified, the user can create a
                            new config via prompts on the command line.

when I do -c with no path, it's supposed to populate the options from my current default config, correct? it doesn't look that way...

for -g

  -g, --createConfigPath    The path of the config to be created via the command
                            line prompt.

I also got the same result - it just shows me the help text again after I answer a question -

image

but it did create c:\temp\config.yaml.

I also noticed in the .yaml there's no longer comments in available_columns. it would be useful to still have these when the user edits the config by hand.

MokoSan commented 2 years ago

it just shows me the help text again

I have removed the help menu display if the user passes in: -c without the path OR -g <path> BUT no process id / process name. This does seem like the more user friendly choice. My previous rationale was that the process id / process name must be passed always or else, we'd display the usage via the help text (was going off @chrisnas' example).

also the selected columns are very difficult to read being in one very long line.. would be great to keep each column as its own line.

This has been fixed. Example:

Prompt: image

Post-Selection: image

I'm unclear what this means -

  -c, --configPath          The path to the YAML configuration file that is read
                            in. If no path is specified, the user can create a
                            new config via prompts on the command line.

I think I have improved the description here but would like a second opinion / any further improvements.

  -c, --configPath          The path to the YAML configuration file that is used
                            in the session; this configuration file contains
                            information such as columns to display that can be
                            configured accordingly. If no path is specified, the
                            default configuration is overwritten by the answers
                            to the prompts in the command line.

when I do -c with no path, it's supposed to populate the options from my current default config, correct? it doesn't look that way...

I think I fixed this. How I intended this to work is, the details from the defaults such as in the case of columns:

columns:
- gen # The Generation
- suspension time (ms) # The time in milliseconds that it took to suspend all threads to start this GC 
- gen0 alloc (mb) # Amount allocated in Gen0 since the last GC occurred in MB.
- gen0 alloc rate  #  The average allocation rate since the last GC.

the prompt would be pre-populated as: image

I also noticed in the .yaml there's no longer comments in available_columns. it would be useful to still have these when the user edits the config by hand.

Have programmatically added as many comments as possible. Example:

# Selected Columns to Display
columns:
- gen # The Generation
- suspension time (ms) # The time in milliseconds that it took to suspend all threads to start this GC 
- gen0 alloc (mb) # Amount allocated in Gen0 since the last GC occurred in MB.
- gen0 alloc rate #  The average allocation rate since the last GC.
# All the Columns Available to Choose From. Please refer to the table via the following link for details about the columns: https://github.com/Maoni0/realmon#configuration
available_columns:
- index # The GC Index.
- type # The Type of GC.
- gen # The Generation
- pause (ms) # The time managed threads were paused during this GC, in milliseconds
- reason # Reason for GC.
- suspension time (ms) # The time in milliseconds that it took to suspend all threads to start this GC 
- pause time (%) # The amount of time that execution in managed code is blocked because the GC needs exclusive use to the heap. For background GCs this is small.
- gen0 alloc (mb) # Amount allocated in Gen0 since the last GC occurred in MB.
- gen0 alloc rate #  The average allocation rate since the last GC.
- peak size (mb) # The size on entry of this GC (includes fragmentation) in MB.
- after size (mb) # The size on exit of this GC (includes fragmentation) in MB.
- peak/after # Peak / After
- promoted (mb) # Memory this GC promoted in MB.
- finalize promoted (mb) # The size of finalizable objects that were discovered to be dead and so promoted during this GC, in MB.
- pinned objects # Number of pinned objects observed in this GC.
- gen0 size (mb) # Size of gen0 at the end of this GC in MB.
- gen0 survival rate # The % of objects in gen0 that survived this GC.
- gen0 frag ratio # The % of fragmentation on gen0 at the end of this GC.
- gen1 size (mb) # Size of gen1 at the end of this GC in MB.
- gen1 survival rate # The % of objects in gen1 that survived this GC.
- gen1 frag ratio # The % of fragmentation on gen1 at the end of this GC.
- gen2 size (mb) # Size of gen2 at the end of this GC in MB.
- gen2 survival rate # The % of objects in gen2 that survived this GC.
- gen2 frag ratio # The % of fragmentation on gen2 at the end of this GC.
- LOH size (mb) # Size of LOH at the end of this GC in MB.
- LOH survival rate # The % of objects in LOH that survived this GC.
- LOH frag ratio # The % of fragmentation on LOH at the end of this GC.
# Statistic Mode including timer period for printing Heap Stats.
stats_mode: 
# Display conditions such as min gc pause time to display.
display_conditions: 
MokoSan commented 2 years ago

@chrisnas @Maoni0, I think this PR is ready to be merged. Please let me know if you think any other changes are needed / have any other feedback.

Thanks for all your help! Been a great learning experience.

chrisnas commented 2 years ago

LGTM! I tried it a little and it seems to work fine. @chrisnas do you have any comments?

If all my comments were taken into account, goooooooooooooo! P.S. great PR :^)

Maoni0 commented 2 years ago

thanks sooo much for doing this! 🙂

Maoni0 commented 2 years ago

I'll publish a new version to nuget tonight

MokoSan commented 2 years ago

image

I double checked the changes and I think we are good. I am going to do some additional testing once the new nuget package is published.

Thanks again for your help, @Maoni0 and @chrisnas!