Re: [PATCH v2 2/2] perf stat: Add support to print counts after a period of time

2018-01-25 Thread Jiri Olsa
On Fri, Jan 26, 2018 at 09:59:02AM +0800, 禹舟键 wrote:
> 2018-01-25 20:27 GMT+08:00 Jiri Olsa :
> 
> > On Thu, Jan 25, 2018 at 10:10:04AM +0100, ufo19890607 wrote:
> >
> > SNIP
> >
> > >  --metric-only::
> > >  Only print computed metrics. Print them in a single line.
> > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > index 406f546ad74c..427f06dc35cc 100644
> > > --- a/tools/perf/builtin-stat.c
> > > +++ b/tools/perf/builtin-stat.c
> > > @@ -573,6 +573,7 @@ static int __run_perf_stat(int argc, const char
> > **argv)
> > >  {
> > >   int interval = stat_config.interval;
> > >   int times = stat_config.times;
> > > + int time = stat_config.time;
> > >   char msg[BUFSIZ];
> > >   unsigned long long t0, t1;
> > >   struct perf_evsel *counter;
> > > @@ -586,6 +587,9 @@ static int __run_perf_stat(int argc, const char
> > **argv)
> > >   if (interval) {
> > >   ts.tv_sec  = interval / USEC_PER_MSEC;
> > >   ts.tv_nsec = (interval % USEC_PER_MSEC) * NSEC_PER_MSEC;
> > > + } else if (time) {
> > > + ts.tv_sec  = time / USEC_PER_MSEC;
> > > + ts.tv_nsec = (time % USEC_PER_MSEC) * NSEC_PER_MSEC;
> >
> > I like the idea.. it won't work with -I option, but let's
> > keep it like that until someone needs it ;-)
> >
> >
> Hi Jirka
> 
> What do you mean by this sentence???

that I'm ok with the patch with small fixes I mentioned

jirka


Re: [PATCH v2 2/2] perf stat: Add support to print counts after a period of time

2018-01-25 Thread Jiri Olsa
On Thu, Jan 25, 2018 at 10:10:04AM +0100, ufo19890607 wrote:

SNIP

>  --metric-only::
>  Only print computed metrics. Print them in a single line.
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 406f546ad74c..427f06dc35cc 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -573,6 +573,7 @@ static int __run_perf_stat(int argc, const char **argv)
>  {
>   int interval = stat_config.interval;
>   int times = stat_config.times;
> + int time = stat_config.time;
>   char msg[BUFSIZ];
>   unsigned long long t0, t1;
>   struct perf_evsel *counter;
> @@ -586,6 +587,9 @@ static int __run_perf_stat(int argc, const char **argv)
>   if (interval) {
>   ts.tv_sec  = interval / USEC_PER_MSEC;
>   ts.tv_nsec = (interval % USEC_PER_MSEC) * NSEC_PER_MSEC;
> + } else if (time) {
> + ts.tv_sec  = time / USEC_PER_MSEC;
> + ts.tv_nsec = (time % USEC_PER_MSEC) * NSEC_PER_MSEC;

I like the idea.. it won't work with -I option, but let's
keep it like that until someone needs it ;-)

>   } else {
>   ts.tv_sec  = 1;
>   ts.tv_nsec = 0;
> @@ -698,9 +702,11 @@ static int __run_perf_stat(int argc, const char **argv)
>   perf_evlist__start_workload(evsel_list);
>   enable_counters();
>  
> - if (interval) {
> + if (interval || time) {
>   while (!waitpid(child_pid, &status, WNOHANG)) {
>   nanosleep(&ts, NULL);
> + if (time)
> + break;
>   process_interval();
>   if (interval_count == true) {
>   if (--times == 0)
> @@ -722,7 +728,9 @@ static int __run_perf_stat(int argc, const char **argv)
>   enable_counters();
>   while (!done) {
>   nanosleep(&ts, NULL);
> - if (interval) {
> + if (interval || time) {
> + if (time)
> + break;

you can put the time check with break directly after nanosleep
to keep some consistency with the workload case


>   process_interval();
>   if (interval_count == true) {
>   if (--times == 0)
> @@ -1904,6 +1912,8 @@ static const struct option stat_options[] = {
>   "print counts at regular interval in ms (>= 10)"),
>   OPT_INTEGER(0, "interval-count", &stat_config.times,
>   "print counts for fixed number of times"),
> + OPT_UINTEGER(0, "time", &stat_config.time,
> + "print counts after a period of time in ms (>= 10)"),
>   OPT_SET_UINT(0, "per-socket", &stat_config.aggr_mode,
>"aggregate counts per processor socket", AGGR_SOCKET),
>   OPT_SET_UINT(0, "per-core", &stat_config.aggr_mode,
> @@ -2701,7 +2711,7 @@ int cmd_stat(int argc, const char **argv)
>   int status = -EINVAL, run_idx;
>   const char *mode;
>   FILE *output = stderr;
> - unsigned int interval;
> + unsigned int interval, time;
>   int times;
>   const char * const stat_subcommands[] = { "record", "report" };
>  
> @@ -2734,6 +2744,7 @@ int cmd_stat(int argc, const char **argv)
>  
>   interval = stat_config.interval;
>   times = stat_config.times;
> + time = stat_config.time;
>  
>   /*
>* For record command the -o is already taken care of.
> @@ -2885,6 +2896,7 @@ int cmd_stat(int argc, const char **argv)
>  "The overhead percentage could be high in 
> some cases. "
>  "Please proceed with caution.\n");
>   }
> +
>   if (times && interval)
>   interval_count = true;
>   else if (times && !interval) {
> @@ -2895,6 +2907,17 @@ int cmd_stat(int argc, const char **argv)
>   goto out;
>   }
>  
> + if (time && time < 10) {
> + pr_err("time must be >= 10ms.\n");

what is this limitation for? please mentioned
that in comment and changelog

thanks,
jirka


[PATCH v2 2/2] perf stat: Add support to print counts after a period of time

2018-01-25 Thread ufo19890607
From: yuzhoujian 

Introduce a new option to print counts after N milliseconds
and update perf-stat documentation accordingly.

Show below is the output of the new option for perf stat.

$ perf stat --time 2000 -e cycles -a
Performance counter stats for 'system wide':

157,260,423  cycles

2.003060766 seconds time elapsed

We can print the count deltas after N milliseconds with this new
introduced option. This option is not supported with "-I" option.

Changes since v1:
- none.

Signed-off-by: yuzhoujian 
---
 tools/perf/Documentation/perf-stat.txt |  7 ++-
 tools/perf/builtin-stat.c  | 29 ++---
 tools/perf/util/stat.h |  1 +
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt 
b/tools/perf/Documentation/perf-stat.txt
index 47a21645f60c..8270a0708ea2 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -149,7 +149,12 @@ The overhead percentage could be high in some cases, for 
instance with small, su
 --interval-count times::
 Print count deltas for fixed number of times.
 This option should be used together with "-I" option.
-   example: 'perf stat -I 1000 --interval-count 2 -e cycles -a'
+   example: 'perf stat -I 1000 --times-print 2 -e cycles -a'
+
+--time msecs::
+Print count deltas after N milliseconds (minimum: 10 ms).
+This option is not supported with "-I" option.
+   example: 'perf stat --time 2000 -e cycles -a'
 
 --metric-only::
 Only print computed metrics. Print them in a single line.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 406f546ad74c..427f06dc35cc 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -573,6 +573,7 @@ static int __run_perf_stat(int argc, const char **argv)
 {
int interval = stat_config.interval;
int times = stat_config.times;
+   int time = stat_config.time;
char msg[BUFSIZ];
unsigned long long t0, t1;
struct perf_evsel *counter;
@@ -586,6 +587,9 @@ static int __run_perf_stat(int argc, const char **argv)
if (interval) {
ts.tv_sec  = interval / USEC_PER_MSEC;
ts.tv_nsec = (interval % USEC_PER_MSEC) * NSEC_PER_MSEC;
+   } else if (time) {
+   ts.tv_sec  = time / USEC_PER_MSEC;
+   ts.tv_nsec = (time % USEC_PER_MSEC) * NSEC_PER_MSEC;
} else {
ts.tv_sec  = 1;
ts.tv_nsec = 0;
@@ -698,9 +702,11 @@ static int __run_perf_stat(int argc, const char **argv)
perf_evlist__start_workload(evsel_list);
enable_counters();
 
-   if (interval) {
+   if (interval || time) {
while (!waitpid(child_pid, &status, WNOHANG)) {
nanosleep(&ts, NULL);
+   if (time)
+   break;
process_interval();
if (interval_count == true) {
if (--times == 0)
@@ -722,7 +728,9 @@ static int __run_perf_stat(int argc, const char **argv)
enable_counters();
while (!done) {
nanosleep(&ts, NULL);
-   if (interval) {
+   if (interval || time) {
+   if (time)
+   break;
process_interval();
if (interval_count == true) {
if (--times == 0)
@@ -1904,6 +1912,8 @@ static const struct option stat_options[] = {
"print counts at regular interval in ms (>= 10)"),
OPT_INTEGER(0, "interval-count", &stat_config.times,
"print counts for fixed number of times"),
+   OPT_UINTEGER(0, "time", &stat_config.time,
+   "print counts after a period of time in ms (>= 10)"),
OPT_SET_UINT(0, "per-socket", &stat_config.aggr_mode,
 "aggregate counts per processor socket", AGGR_SOCKET),
OPT_SET_UINT(0, "per-core", &stat_config.aggr_mode,
@@ -2701,7 +2711,7 @@ int cmd_stat(int argc, const char **argv)
int status = -EINVAL, run_idx;
const char *mode;
FILE *output = stderr;
-   unsigned int interval;
+   unsigned int interval, time;
int times;
const char * const stat_subcommands[] = { "record", "report" };
 
@@ -2734,6 +2744,7 @@ int cmd_stat(int argc, const char **argv)
 
interval = stat_config.interval;
times = stat_config.times;
+   time = stat_config.time;
 
/*
 * For record command the -o is already taken care of.
@@ -2885,6 +2896,7 @@ int cmd_stat(int argc, const char **argv)
   "The over