Closed MokoSan closed 2 years ago
great!! I will take a look at this later today :)
this looks great! one suggestion, when we read in the columns in the .yaml, should we do some verification to make sure these are valid columns, ie, the user didn't accidently type in a name for a column that doesn't exist?
this looks great! one suggestion, when we read in the columns in the .yaml, should we do some verification to make sure these are valid columns, ie, the user didn't accidently type in a name for a column that doesn't exist?
Thanks for the great suggestion! Added a bunch of validation and unit tests to ensure the right config is loaded.
After these changes are merged in, I was thinking about implementing the following features:
Timer based heap stats + config changes with Nick's changes.
Adding more columns in the config such as:
Low pri since we have a default config available but adding an interactive way to create the config such as the following experience:
Users get the following prompt to choose which columns they'd like to add after they supply a command line parameter to create a config:
Once the columns they care about are selected, they are automatically written to a file:
Full Config file:
columns:
# optional columns to add. The default one is always: `index` or GC#.
- gen
- pause (ms)
available_columns:
# all columns available - could be many here since TraceGC exposes lots of stuff
- gen # Number of Generation
- type # Type of GC
- pause (ms) # GC Pause Time in MSec
- reason # GC Reason
Just my 2 cents, M.
your ideas all sound fine to me :) for more columns in the config file, you could take a look at this slightly different place in gcstats.cs, this is the code that displays the GCStats tables in PerfView. it's a bit better than the ToCSV method - this reminds me that ToCSV
needs a fix which is it's not getting the allocGen0MB
correctly. the way we are getting it in PrintEventTable
, ie, UserAllocated[(int)Gens.Gen0]
is correct. BTW this is actually a great column to display along with UserAllocated[(int)Gens.GenLargeObj]
- they show how much user allocations were done between this and last GC which are things that users really care about.
for interactively choosing the column names, could we also display what's in the comment so for users who may not be familiar with what these columns mean, they can get an explanation :)
Added the following major changes to the repo:
1. Added a YAML based Configuration file and a Corresponding Reader
The current contents of this file contains columns to display and all available columns. A new command line parameter
-c
is added to specify the path of the said yaml config file. By default, it reads theDefaultConfig.yaml
file that's a part of the repository. A corresponding reader utility is also added to read in this config - this parser assumes the underscore naming convention.The current contents of the config file are:
The next step with the config file is to add heap stat based configuration such as a timer based retrieval of the heap stats.
2. Dynamically Adding Columns
To make it easier to add and use new columns, a
ColumnInfoMap
has been added where the dev simply registers the column to add by implementing aColumnInfo
object. The steps to do this are:available_columns
in the Default.yaml config.ColumnInfo
object in theColumnInfoMap
with the following properties:Func<TraceGC, object>
that looks up an object in via aTraceGC
event.Once the dev follows these steps and adds the new column to the columns config, the new data based on the column should automatically show up. For the future, the methods implemented to retrieve the format based on the dynamic columns will help with outputting the result in different formats such as a text file rather than outputting to the command line.
Unit Tests
Unit tests have been added for a majority (if not all) of the utility functions and the config reader.