Write thoughput data of ddsperf to csv file specified on command line via -f option #583
Conversation
… via -f option Signed-off-by: Bart <bart.poot@adlinktech.com>
There was a problem hiding this comment.
Thanks @BartP6703, not logging the data in an easily processed format and instead parsing the console output definitely was a mistake and so this is a welcome addition. However, it would be much better do a bit of additional work and log all the interesting data, rather than just this tiny subset.
src/tools/ddsperf/ddsperf.c
Outdated
| @@ -1381,12 +1384,18 @@ struct dds_stats { | |||
| const struct dds_stat_keyvalue *discarded_bytes; | |||
| }; | |||
|
|
|||
| static bool print_stats (dds_time_t tref, dds_time_t tnow, dds_time_t tprev, struct record_cputime_state *cputime_state, struct record_netload_state *netload_state, struct dds_stats *stats) | |||
| static bool print_stats (dds_time_t tref, dds_time_t tnow, dds_time_t tprev, struct record_cputime_state *cputime_state, struct record_netload_state *netload_state, struct dds_stats *stats, char *filename) | |||
There was a problem hiding this comment.
I don't think it makes much sense to pass in the filename here when the file is a global variable. It would be more in line with the rest of the code to open the file in main, then use it here. (Or, alternatively, to make file local to main and pass it in here.)
There was a problem hiding this comment.
Used the filename in optarg to open the file and use the (global) file descriptor.
src/tools/ddsperf/ddsperf.c
Outdated
| { | ||
| int64_t tsec = (int64_t)(tnow / 1000000000); | ||
| int64_t nsec = (int64_t)(tnow % 1000000000); | ||
| fprintf (file, "%"PRId64".%"PRId64",%s,%"PRIdPID",%.3f,%"PRIu32",%.2f,%.2f\n", |
There was a problem hiding this comment.
That should be "%"PRId64".%06"PRId64" and I suspect the %s should be \"%s\" but I don't know CSV rules very well.
I furthermore think some of the other fields that get written to stdout here are useful; and certainly the CPU, memory and network load data would be good to also collect in the CSV file. They are gathered and printed because they are valuable for understanding what is going on, it follows that a log file specifically for analysis purposes should include them.
P.S. There is also data output for latency measurements, that should be logged as well.
There was a problem hiding this comment.
- Changed the format specifier as proposed in the comment.
- Added code to log the latency measurements to csv file.
src/tools/ddsperf/ddsperf.c
Outdated
| @@ -1939,11 +1963,12 @@ int main (int argc, char *argv[]) | |||
| char netload_if[256]; | |||
| double netload_bw = -1; | |||
| double rss_init = 0.0, rss_final = 0.0; | |||
| char filename[256] = { 0 }; | |||
There was a problem hiding this comment.
There's no need to copy the file name to the stack, one can just store optarg.
There was a problem hiding this comment.
Used optarg to open the file.
…pecified on the command line of ddsperf). Signed-off-by: Bart <bart.poot@adlinktech.com>
Signed-off-by: Bart <bart.poot@adlinktech.com>
Added extra newline at the end of the csv file just before closing it Signed-off-by: Bart <bart.poot@adlinktech.com>
Signed-off-by: Bart <bart.poot@adlinktech.com>
Signed-off-by: Bart bart.poot@adlinktech.com