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

Write thoughput data of ddsperf to csv file specified on command line via -f option #583

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions src/tools/ddsperf/ddsperf.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@

#define PINGPONG_RAWSIZE 20000

/* File descriptor to write ddsperf data to */
static FILE *file = NULL;

enum topicsel {
KS, /* KeyedSeq type: seq#, key, sequence-of-octet */
K32, /* Keyed32 type: seq#, key, array-of-24-octet (sizeof = 32) */
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor Author

@BartP6703 BartP6703 Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used the filename in optarg to open the file and use the (global) file descriptor.

{
char prefix[128];
const double ts = (double) (tnow - tref) / 1e9;
bool output = false;
snprintf (prefix, sizeof (prefix), "[%"PRIdPID"] %.3f ", ddsrt_getpid (), ts);
ddsrt_pid_t pid = ddsrt_getpid ();
char hostname[64];
if (ddsrt_gethostname (hostname, sizeof (hostname)) != DDS_RETCODE_OK)
{
snprintf (hostname, sizeof (hostname), "%s", "<unknown>");
}
snprintf (prefix, sizeof (prefix), "[%"PRIdPID"] %.3f ", pid, ts);

if (pub_rate > 0)
{
Expand Down Expand Up @@ -1427,11 +1436,25 @@ static bool print_stats (dds_time_t tref, dds_time_t tnow, dds_time_t tprev, str

if (nrecv > 0 || substat_every_second)
{
if (filename != NULL)
{
if (file == NULL)
{
file = fopen(filename, "w");
}
}
const double dt = (double) (tnow - tprev);
printf ("%s size %"PRIu32" total %"PRIu64" lost %"PRIu64" delta %"PRIu64" lost %"PRIu64" rate %.2f kS/s %.2f Mb/s (%.2f kS/s %.2f Mb/s)\n",
prefix, last_size, tot_nrecv, tot_nlost, nrecv, nlost,
(double) nrecv * 1e6 / dt, (double) nrecv_bytes * 8 * 1e3 / dt,
(double) nrecv10s * 1e6 / (10 * dt), (double) nrecv10s_bytes * 8 * 1e3 / (10 * dt));
if (file != NULL)
{
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",
Copy link
Contributor

@eboasson eboasson Aug 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Changed the format specifier as proposed in the comment.
  • Added code to log the latency measurements to csv file.

tsec, nsec / 1000, hostname, pid, ts, last_size, (double) nrecv_bytes * 8 * 1e3 / dt, (double) nrecv10s_bytes * 8 * 1e3 / (10 * dt));
}
output = true;
}
}
Expand Down Expand Up @@ -1576,7 +1599,7 @@ static void sigxfsz_handler (int sig __attribute__ ((unused)))
if (write (2, msg, sizeof (msg) - 1) < 0) {
/* may not ignore return value according to Linux/gcc */
}
print_stats (0, tnow, tnow - DDS_SECS (1), NULL, NULL, NULL);
print_stats (0, tnow, tnow - DDS_SECS (1), NULL, NULL, NULL, NULL);
kill (getpid (), 9);
}
}
Expand Down Expand Up @@ -1635,6 +1658,7 @@ OPTIONS:\n\
data\n\
-X output extended statistics\n\
-i ID use domain ID instead of the default domain\n\
-f <file> write output to <file>\n\
\n\
MODE... is zero or more of:\n\
ping [R[Hz]] [size S] [waitset|listener]\n\
Expand Down Expand Up @@ -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 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to copy the file name to the stack, one can just store optarg.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used optarg to open the file.

ddsrt_threadattr_init (&attr);

argv0 = argv[0];

while ((opt = getopt (argc, argv, "1cd:D:i:n:k:uLK:T:Q:R:Xh")) != EOF)
while ((opt = getopt (argc, argv, "1cd:D:i:n:k:uLK:T:Q:R:Xhf:")) != EOF)
{
int pos;
switch (opt)
Expand All @@ -1965,6 +1990,7 @@ int main (int argc, char *argv[])
}
case 'D': dur = atof (optarg); if (dur <= 0) dur = HUGE_VAL; break;
case 'i': did = (dds_domainid_t) atoi (optarg); break;
case 'f': (void) ddsrt_strlcpy (filename, optarg, sizeof (filename)); break;
case 'n': nkeyvals = (unsigned) atoi (optarg); break;
case 'u': reliable = false; break;
case 'k': histdepth = atoi (optarg); if (histdepth < 0) histdepth = 0; break;
Expand Down Expand Up @@ -2417,7 +2443,7 @@ int main (int argc, char *argv[])
if (tnext <= tnow)
{
bool output;
output = print_stats (tref, tnow, tlast, cputime_state, netload_state, &stats);
output = print_stats (tref, tnow, tlast, cputime_state, netload_state, &stats, filename);
tlast = tnow;
if (tnow > tnext + DDS_MSECS (500))
tnext = tnow + DDS_SECS (1);
Expand Down Expand Up @@ -2566,5 +2592,9 @@ int main (int argc, char *argv[])
printf ("[%"PRIdPID"] error: RSS grew too much (%f -> %f)\n", ddsrt_getpid (), rss_init, rss_final);
ok = false;
}
if (file != NULL)
{
fclose(file);
}
return ok ? 0 : 1;
}