Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-12-05 Thread Jeremie Courreges-Anglas 
On Tue, Dec 05 2017, Scott Cheloha  wrote:
> Hey,
>
> Sorry for the delay.
>
> On Sun, Nov 26, 2017 at 07:00:36PM +0100, Jeremie Courreges-Anglas wrote:
>> On Sun, Nov 26 2017, Jeremie Courreges-Anglas  wrote:
>> > On Sat, Nov 25 2017, Brent Cook  wrote:
>> >> Thanks guys. This will make enabling this on the odder platforms in
>> >> portable easier.
>> >
>> > NB: if we want to able to mix app_tminterval() for real and user time, the
>> > static storage used for "start" should be different.  This is not the
>> > case for the Windows implementation, which *seems* to support only user
>> > time:
>> >
>> >   
>> > https://github.com/libressl-portable/portable/blob/master/apps/openssl/apps_win.c
>> >
>> > Another approach, which would require more changes, would be to just
>> > provide two seperate functions, possibly named app_timer_real() and
>> > app_timer_user().  The Windows version of app_timer_real() could start
>> > as a simple copy of app_timer_user(), marked with XXX...
>> 
>> So here's a diff similar to what Scott proposed in his first diff.
>> 
>> The apps_win.c diff for portable would look like this
>> (untested):
>> 
>>   https://pbot.rmdir.de/sy3XxnKuTQHgotB35BJ6vQ
>
> That looks fine to me, but I prefer the names you gave above,
> "app_timer_real" and "app_timer_user".

Fine with me.

>> I resisted the temptation to make use of TM_START/TM_STOP instead of
>> local defines, this could be another TODO entry.  I don't like the
>> TM_START/TM_STOP names btw, TM_STOP doesn't actually stop the timer,
>> it's just a "get" operation.
>
> Agreed re. the operation names.  Can we call them TM_SET/TM_GET?
> One does "set" a timer.

I'd suggest TM_RESET and TM_GET, "GET" and "SET" don't look very
different when reading the code.  Diff below.

>> Do you folks agree with this approach?
>
> Assuming you feel the same way about what you wrote two weeks ago,
> yeah I'm good with that.
>
> I'll send a pull request with a mix of your changes and an
> implementation for app_timer_user() on Windows to libressl-portable
> today or tomorrow and reply here with the same.
>
> A subsequent diff will expose the separate functions to the rest of
> the modules via apps.h, and later diffs in, e.g., speed and s_time,
> can make use of the now-distinct timer routines at their leisure.

I went ahead and committed the diff to make both functions available to
the cvs tree.  LibreSSL development happens in base, LibreSSL-portable
then uses that code, so you need only provide a patch for apps_win.c.
Maybe discuss this with Brent (cc'd).

With the diff below you should be able to focus on reusing
app_timer_user/real.

Thoughts?  oks?


Index: apps.h
===
--- apps.h.orig
+++ apps.h
@@ -277,8 +277,8 @@ unsigned char *next_protos_parse(unsigne
 
 int app_isdir(const char *);
 
-#define TM_START   0
-#define TM_STOP1
+#define TM_RESET   0
+#define TM_GET 1
 double app_timer_real(int stop);
 double app_timer_user(int stop);
 
Index: apps_posix.c
===
--- apps_posix.c.orig
+++ apps_posix.c
@@ -124,13 +124,13 @@
 #include "apps.h"
 
 double
-app_timer_real(int stop)
+app_timer_real(int op)
 {
static struct timespec start;
struct timespec elapsed, now;
 
clock_gettime(CLOCK_MONOTONIC, );
-   if (stop) {
+   if (op == TM_GET) {
timespecsub(, , );
return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
}
@@ -139,14 +139,14 @@ app_timer_real(int stop)
 }
 
 double
-app_timer_user(int stop)
+app_timer_user(int op)
 {
static struct timeval start;
struct timeval elapsed;
struct rusage now;
 
getrusage(RUSAGE_SELF, );
-   if (stop) {
+   if (op == TM_GET) {
timersub(_utime, , );
return elapsed.tv_sec + elapsed.tv_usec / 100.0;
}
Index: s_time.c
===
--- s_time.c.orig
+++ s_time.c
@@ -229,9 +229,6 @@ s_time_usage(void)
 /***
  * TIME - time functions
  */
-#define START  0
-#define STOP   1
-
 static double
 tm_Time_F(int op)
 {
@@ -331,7 +328,7 @@ s_time_main(int argc, char **argv)
 
bytes_read = 0;
finishtime = time(NULL) + s_time_config.maxtime;
-   tm_Time_F(START);
+   tm_Time_F(TM_RESET);
for (;;) {
if (finishtime < time(NULL))
break;
@@ -377,7 +374,7 @@ s_time_main(int argc, char **argv)
SSL_free(scon);
scon = NULL;
}
-   totalTime += tm_Time_F(STOP);   /* Add the time for this iteration */
+   totalTime += tm_Time_F(TM_GET); /* Add the time for this iteration */
 
printf("\n\n%d connections in %.2fs; %.2f

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-12-05 Thread Scott Cheloha 
Hey,

Sorry for the delay.

On Sun, Nov 26, 2017 at 07:00:36PM +0100, Jeremie Courreges-Anglas wrote:
> On Sun, Nov 26 2017, Jeremie Courreges-Anglas  wrote:
> > On Sat, Nov 25 2017, Brent Cook  wrote:
> >> Thanks guys. This will make enabling this on the odder platforms in
> >> portable easier.
> >
> > NB: if we want to able to mix app_tminterval() for real and user time, the
> > static storage used for "start" should be different.  This is not the
> > case for the Windows implementation, which *seems* to support only user
> > time:
> >
> >   
> > https://github.com/libressl-portable/portable/blob/master/apps/openssl/apps_win.c
> >
> > Another approach, which would require more changes, would be to just
> > provide two seperate functions, possibly named app_timer_real() and
> > app_timer_user().  The Windows version of app_timer_real() could start
> > as a simple copy of app_timer_user(), marked with XXX...
> 
> So here's a diff similar to what Scott proposed in his first diff.
> 
> The apps_win.c diff for portable would look like this
> (untested):
> 
>   https://pbot.rmdir.de/sy3XxnKuTQHgotB35BJ6vQ

That looks fine to me, but I prefer the names you gave above,
"app_timer_real" and "app_timer_user".

> I resisted the temptation to make use of TM_START/TM_STOP instead of
> local defines, this could be another TODO entry.  I don't like the
> TM_START/TM_STOP names btw, TM_STOP doesn't actually stop the timer,
> it's just a "get" operation.

Agreed re. the operation names.  Can we call them TM_SET/TM_GET?
One does "set" a timer.

> Do you folks agree with this approach?

Assuming you feel the same way about what you wrote two weeks ago,
yeah I'm good with that.

I'll send a pull request with a mix of your changes and an
implementation for app_timer_user() on Windows to libressl-portable
today or tomorrow and reply here with the same.

A subsequent diff will expose the separate functions to the rest of
the modules via apps.h, and later diffs in, e.g., speed and s_time,
can make use of the now-distinct timer routines at their leisure.

--
Scott Cheloha

> Index: apps.h
> ===
> RCS file: /d/cvs/src/usr.bin/openssl/apps.h,v
> retrieving revision 1.19
> diff -u -p -r1.19 apps.h
> --- apps.h30 Aug 2016 14:34:59 -  1.19
> +++ apps.h26 Nov 2017 17:46:01 -
> @@ -279,7 +279,8 @@ int app_isdir(const char *);
>  
>  #define TM_START 0
>  #define TM_STOP  1
> -double app_tminterval (int stop, int usertime);
> +double app_timer_realtime(int stop);
> +double app_timer_usertime(int stop);
>  
>  #define OPENSSL_NO_SSL_INTERN
>  
> Index: apps_posix.c
> ===
> RCS file: /d/cvs/src/usr.bin/openssl/apps_posix.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 apps_posix.c
> --- apps_posix.c  24 Nov 2017 13:48:12 -  1.3
> +++ apps_posix.c  26 Nov 2017 17:43:18 -
> @@ -123,8 +123,8 @@
>  
>  #include "apps.h"
>  
> -static double
> -real_interval(int stop)
> +double
> +app_timer_realtime(int stop)
>  {
>   static struct timespec start;
>   struct timespec elapsed, now;
> @@ -138,8 +138,8 @@ real_interval(int stop)
>   return 0.0;
>  }
>  
> -static double
> -user_interval(int stop)
> +double
> +app_timer_usertime(int stop)
>  {
>   static struct timeval start;
>   struct timeval elapsed;
> @@ -152,12 +152,6 @@ user_interval(int stop)
>   }
>   start = now.ru_utime;
>   return 0.0;
> -}
> -
> -double
> -app_tminterval(int stop, int usertime)
> -{
> - return (usertime) ? user_interval(stop) : real_interval(stop);
>  }
>  
>  int
> Index: s_time.c
> ===
> RCS file: /d/cvs/src/usr.bin/openssl/s_time.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 s_time.c
> --- s_time.c  2 Nov 2017 00:31:49 -   1.18
> +++ s_time.c  26 Nov 2017 17:40:54 -
> @@ -233,9 +233,9 @@ s_time_usage(void)
>  #define STOP 1
>  
>  static double
> -tm_Time_F(int s)
> +tm_Time_F(int op)
>  {
> - return app_tminterval(s, 1);
> + return app_timer_usertime(op);
>  }
>  
>  /***
> Index: speed.c
> ===
> RCS file: /d/cvs/src/usr.bin/openssl/speed.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 speed.c
> --- speed.c   7 Oct 2017 06:16:54 -   1.20
> +++ speed.c   26 Nov 2017 17:45:29 -
> @@ -202,7 +202,10 @@ sig_done(int sig)
>  static double
>  Time_F(int s)
>  {
> - return app_tminterval(s, usertime);
> + if (usertime)
> + return app_timer_usertime(s);
> + else
> + return app_timer_realtime(s);
>  }
>  
>  
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-26 Thread Jeremie Courreges-Anglas 
On Sun, Nov 26 2017, Jeremie Courreges-Anglas  wrote:
> On Sat, Nov 25 2017, Brent Cook  wrote:
>> Thanks guys. This will make enabling this on the odder platforms in
>> portable easier.
>
> NB: if we want to able to mix app_tminterval() for real and user time, the
> static storage used for "start" should be different.  This is not the
> case for the Windows implementation, which *seems* to support only user
> time:
>
>   
> https://github.com/libressl-portable/portable/blob/master/apps/openssl/apps_win.c
>
> Another approach, which would require more changes, would be to just
> provide two seperate functions, possibly named app_timer_real() and
> app_timer_user().  The Windows version of app_timer_real() could start
> as a simple copy of app_timer_user(), marked with XXX...

So here's a diff similar to what Scott proposed in his first diff.

The apps_win.c diff for portable would look like this
(untested):

  https://pbot.rmdir.de/sy3XxnKuTQHgotB35BJ6vQ

I resisted the temptation to make use of TM_START/TM_STOP instead of
local defines, this could be another TODO entry.  I don't like the
TM_START/TM_STOP names btw, TM_STOP doesn't actually stop the timer,
it's just a "get" operation.

Do you folks agree with this approach?


Index: apps.h
===
RCS file: /d/cvs/src/usr.bin/openssl/apps.h,v
retrieving revision 1.19
diff -u -p -r1.19 apps.h
--- apps.h  30 Aug 2016 14:34:59 -  1.19
+++ apps.h  26 Nov 2017 17:46:01 -
@@ -279,7 +279,8 @@ int app_isdir(const char *);
 
 #define TM_START   0
 #define TM_STOP1
-double app_tminterval (int stop, int usertime);
+double app_timer_realtime(int stop);
+double app_timer_usertime(int stop);
 
 #define OPENSSL_NO_SSL_INTERN
 
Index: apps_posix.c
===
RCS file: /d/cvs/src/usr.bin/openssl/apps_posix.c,v
retrieving revision 1.3
diff -u -p -r1.3 apps_posix.c
--- apps_posix.c24 Nov 2017 13:48:12 -  1.3
+++ apps_posix.c26 Nov 2017 17:43:18 -
@@ -123,8 +123,8 @@
 
 #include "apps.h"
 
-static double
-real_interval(int stop)
+double
+app_timer_realtime(int stop)
 {
static struct timespec start;
struct timespec elapsed, now;
@@ -138,8 +138,8 @@ real_interval(int stop)
return 0.0;
 }
 
-static double
-user_interval(int stop)
+double
+app_timer_usertime(int stop)
 {
static struct timeval start;
struct timeval elapsed;
@@ -152,12 +152,6 @@ user_interval(int stop)
}
start = now.ru_utime;
return 0.0;
-}
-
-double
-app_tminterval(int stop, int usertime)
-{
-   return (usertime) ? user_interval(stop) : real_interval(stop);
 }
 
 int
Index: s_time.c
===
RCS file: /d/cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.18
diff -u -p -r1.18 s_time.c
--- s_time.c2 Nov 2017 00:31:49 -   1.18
+++ s_time.c26 Nov 2017 17:40:54 -
@@ -233,9 +233,9 @@ s_time_usage(void)
 #define STOP   1
 
 static double
-tm_Time_F(int s)
+tm_Time_F(int op)
 {
-   return app_tminterval(s, 1);
+   return app_timer_usertime(op);
 }
 
 /***
Index: speed.c
===
RCS file: /d/cvs/src/usr.bin/openssl/speed.c,v
retrieving revision 1.20
diff -u -p -r1.20 speed.c
--- speed.c 7 Oct 2017 06:16:54 -   1.20
+++ speed.c 26 Nov 2017 17:45:29 -
@@ -202,7 +202,10 @@ sig_done(int sig)
 static double
 Time_F(int s)
 {
-   return app_tminterval(s, usertime);
+   if (usertime)
+   return app_timer_usertime(s);
+   else
+   return app_timer_realtime(s);
 }
 
 


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-26 Thread Jeremie Courreges-Anglas 
On Sat, Nov 25 2017, Brent Cook  wrote:
> Thanks guys. This will make enabling this on the odder platforms in
> portable easier.

NB: if we want to able to mix app_tminterval() for real and user time, the
static storage used for "start" should be different.  This is not the
case for the Windows implementation, which *seems* to support only user
time:

  
https://github.com/libressl-portable/portable/blob/master/apps/openssl/apps_win.c

Another approach, which would require more changes, would be to just
provide two seperate functions, possibly named app_timer_real() and
app_timer_user().  The Windows version of app_timer_real() could start
as a simple copy of app_timer_user(), marked with XXX...

> On Fri, Nov 24, 2017 at 7:03 AM, Scott Cheloha 
> wrote:
>
>> > On Nov 24, 2017, at 6:58 AM, Jeremie Courreges-Anglas 
>> wrote:
>> >
>> > On Wed, Nov 22 2017, Scott Cheloha  wrote:
>> >> Whoops, ignore that last patch, it lacked the
>> >> static changes in apps_posix.c
>> >
>> > This looks good to me.  I'm tempted to commit the apps_posix.c part
>> > first: it seems to me that app_tminterval() could be reused in s_time.c,
>> > leading to simpler code instead of inlining clock_gettime calls.
>>
>> I intend to refactor that module next.  One thing I was going to
>> do was abstract away the timer interface, so that works.
>>
>> --
>> Scott Cheloha
>>
>>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-25 Thread Brent Cook 
Thanks guys. This will make enabling this on the odder platforms in
portable easier.

On Fri, Nov 24, 2017 at 7:03 AM, Scott Cheloha 
wrote:

> > On Nov 24, 2017, at 6:58 AM, Jeremie Courreges-Anglas 
> wrote:
> >
> > On Wed, Nov 22 2017, Scott Cheloha  wrote:
> >> Whoops, ignore that last patch, it lacked the
> >> static changes in apps_posix.c
> >
> > This looks good to me.  I'm tempted to commit the apps_posix.c part
> > first: it seems to me that app_tminterval() could be reused in s_time.c,
> > leading to simpler code instead of inlining clock_gettime calls.
>
> I intend to refactor that module next.  One thing I was going to
> do was abstract away the timer interface, so that works.
>
> --
> Scott Cheloha
>
>

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-24 Thread Scott Cheloha 
> On Nov 24, 2017, at 6:58 AM, Jeremie Courreges-Anglas  wrote:
> 
> On Wed, Nov 22 2017, Scott Cheloha  wrote:
>> Whoops, ignore that last patch, it lacked the
>> static changes in apps_posix.c
> 
> This looks good to me.  I'm tempted to commit the apps_posix.c part
> first: it seems to me that app_tminterval() could be reused in s_time.c,
> leading to simpler code instead of inlining clock_gettime calls.

I intend to refactor that module next.  One thing I was going to
do was abstract away the timer interface, so that works.

--
Scott Cheloha

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-24 Thread Jeremie Courreges-Anglas 
On Wed, Nov 22 2017, Scott Cheloha  wrote:
> Whoops, ignore that last patch, it lacked the
> static changes in apps_posix.c

This looks good to me.  I'm tempted to commit the apps_posix.c part
first: it seems to me that app_tminterval() could be reused in s_time.c,
leading to simpler code instead of inlining clock_gettime calls.

> --
> Scott Cheloha
>
> Index: usr.bin/openssl/apps_posix.c
> ===
> RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 apps_posix.c
> --- usr.bin/openssl/apps_posix.c  13 Sep 2015 12:41:01 -  1.2
> +++ usr.bin/openssl/apps_posix.c  22 Nov 2017 16:39:42 -
> @@ -116,31 +116,48 @@
>   * Functions that need to be overridden by non-POSIX operating systems.
>   */
>  
> -#include 
> +#include 
> +#include 
>  
> -#include 
> +#include 
>  
>  #include "apps.h"
>  
> -double
> -app_tminterval(int stop, int usertime)
> +static double
> +real_interval(int stop)
>  {
> - double ret = 0;
> - struct tms rus;
> - clock_t now = times();
> - static clock_t tmstart;
> -
> - if (usertime)
> - now = rus.tms_utime;
> -
> - if (stop == TM_START)
> - tmstart = now;
> - else {
> - long int tck = sysconf(_SC_CLK_TCK);
> - ret = (now - tmstart) / (double) tck;
> + static struct timespec start;
> + struct timespec elapsed, now;
> +
> + clock_gettime(CLOCK_MONOTONIC, );
> + if (stop) {
> + timespecsub(, , );
> + return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
>   }
> + start = now;
> + return 0.0;
> +}
>  
> - return (ret);
> +static double
> +user_interval(int stop)
> +{
> + static struct timeval start;
> + struct timeval elapsed;
> + struct rusage now;
> +
> + getrusage(RUSAGE_SELF, );
> + if (stop) {
> + timersub(_utime, , );
> + return elapsed.tv_sec + elapsed.tv_usec / 100.0;
> + }
> + start = now.ru_utime;
> + return 0.0;
> +}
> +
> +double
> +app_tminterval(int stop, int usertime)
> +{
> + return (usertime) ? user_interval(stop) : real_interval(stop);
>  }
>  
>  int
> Index: usr.bin/openssl/s_time.c
> ===
> RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 s_time.c
> --- usr.bin/openssl/s_time.c  2 Nov 2017 00:31:49 -   1.18
> +++ usr.bin/openssl/s_time.c  22 Nov 2017 16:39:42 -
> @@ -63,11 +63,13 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -248,7 +250,7 @@ s_time_main(int argc, char **argv)
>   double totalTime = 0.0;
>   int nConn = 0;
>   SSL *scon = NULL;
> - time_t finishtime;
> + struct timespec finishtime, now;
>   int ret = 1;
>   char buf[1024 * 8];
>   int ver;
> @@ -330,10 +332,12 @@ s_time_main(int argc, char **argv)
>   /* Loop and time how long it takes to make connections */
>  
>   bytes_read = 0;
> - finishtime = time(NULL) + s_time_config.maxtime;
> + clock_gettime(CLOCK_MONOTONIC, );
> + finishtime.tv_sec += s_time_config.maxtime;
>   tm_Time_F(START);
>   for (;;) {
> - if (finishtime < time(NULL))
> + clock_gettime(CLOCK_MONOTONIC, );
> + if (timespeccmp(, , <))
>   break;
>   if ((scon = doConnection(NULL)) == NULL)
>   goto end;
> @@ -383,7 +387,7 @@ s_time_main(int argc, char **argv)
>   nConn, totalTime, ((double) nConn / totalTime), bytes_read);
>   printf("%d connections in %lld real seconds, %ld bytes read per 
> connection\n",
>   nConn,
> - (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> + (long long)(now.tv_sec - finishtime.tv_sec + s_time_config.maxtime),
>   bytes_read / nConn);
>  
>   /*
> @@ -422,14 +426,16 @@ next:
>   nConn = 0;
>   totalTime = 0.0;
>  
> - finishtime = time(NULL) + s_time_config.maxtime;
> + clock_gettime(CLOCK_MONOTONIC, );
> + finishtime.tv_sec += s_time_config.maxtime;
>  
>   printf("starting\n");
>   bytes_read = 0;
>   tm_Time_F(START);
>  
>   for (;;) {
> - if (finishtime < time(NULL))
> + clock_gettime(CLOCK_MONOTONIC, );
> + if (timespeccmp(, , <))
>   break;
>   if ((doConnection(scon)) == NULL)
>   goto end;
> @@ -475,7 +481,7 @@ next:
>   printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes 
> read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
>   printf("%d connections in %lld real seconds, %ld bytes read per 
> connection\n",
>   nConn,
> - (long long)(time(NULL) -

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-22 Thread Scott Cheloha 
Whoops, ignore that last patch, it lacked the
static changes in apps_posix.c

--
Scott Cheloha

Index: usr.bin/openssl/apps_posix.c
===
RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
retrieving revision 1.2
diff -u -p -r1.2 apps_posix.c
--- usr.bin/openssl/apps_posix.c13 Sep 2015 12:41:01 -  1.2
+++ usr.bin/openssl/apps_posix.c22 Nov 2017 16:39:42 -
@@ -116,31 +116,48 @@
  * Functions that need to be overridden by non-POSIX operating systems.
  */
 
-#include 
+#include 
+#include 
 
-#include 
+#include 
 
 #include "apps.h"
 
-double
-app_tminterval(int stop, int usertime)
+static double
+real_interval(int stop)
 {
-   double ret = 0;
-   struct tms rus;
-   clock_t now = times();
-   static clock_t tmstart;
-
-   if (usertime)
-   now = rus.tms_utime;
-
-   if (stop == TM_START)
-   tmstart = now;
-   else {
-   long int tck = sysconf(_SC_CLK_TCK);
-   ret = (now - tmstart) / (double) tck;
+   static struct timespec start;
+   struct timespec elapsed, now;
+
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (stop) {
+   timespecsub(, , );
+   return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
}
+   start = now;
+   return 0.0;
+}
 
-   return (ret);
+static double
+user_interval(int stop)
+{
+   static struct timeval start;
+   struct timeval elapsed;
+   struct rusage now;
+
+   getrusage(RUSAGE_SELF, );
+   if (stop) {
+   timersub(_utime, , );
+   return elapsed.tv_sec + elapsed.tv_usec / 100.0;
+   }
+   start = now.ru_utime;
+   return 0.0;
+}
+
+double
+app_tminterval(int stop, int usertime)
+{
+   return (usertime) ? user_interval(stop) : real_interval(stop);
 }
 
 int
Index: usr.bin/openssl/s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.18
diff -u -p -r1.18 s_time.c
--- usr.bin/openssl/s_time.c2 Nov 2017 00:31:49 -   1.18
+++ usr.bin/openssl/s_time.c22 Nov 2017 16:39:42 -
@@ -63,11 +63,13 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -248,7 +250,7 @@ s_time_main(int argc, char **argv)
double totalTime = 0.0;
int nConn = 0;
SSL *scon = NULL;
-   time_t finishtime;
+   struct timespec finishtime, now;
int ret = 1;
char buf[1024 * 8];
int ver;
@@ -330,10 +332,12 @@ s_time_main(int argc, char **argv)
/* Loop and time how long it takes to make connections */
 
bytes_read = 0;
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
tm_Time_F(START);
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((scon = doConnection(NULL)) == NULL)
goto end;
@@ -383,7 +387,7 @@ s_time_main(int argc, char **argv)
nConn, totalTime, ((double) nConn / totalTime), bytes_read);
printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
nConn,
-   (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+   (long long)(now.tv_sec - finishtime.tv_sec + s_time_config.maxtime),
bytes_read / nConn);
 
/*
@@ -422,14 +426,16 @@ next:
nConn = 0;
totalTime = 0.0;
 
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
 
printf("starting\n");
bytes_read = 0;
tm_Time_F(START);
 
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((doConnection(scon)) == NULL)
goto end;
@@ -475,7 +481,7 @@ next:
printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes 
read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
nConn,
-   (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+   (long long)(now.tv_sec - finishtime.tv_sec + s_time_config.maxtime),
bytes_read / nConn);
 
ret = 0;

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-22 Thread Scott Cheloha 
On Sat, Nov 18, 2017 at 05:27:14PM +0100, Jeremie Courreges-Anglas wrote:
> On Sat, Nov 11 2017, Scott Cheloha  wrote:
> > [...]
> 
> I doubt that timersub/timespecsub are a big problem to add to -portable,
> they're just macros.  clock_gettime and getrusage seem to already be
> used in libressl(-portable), there's autoconf glue for the former at
> least.

Sounds good to me.

> > Thoughts and feedback?
> 
> More comment inline.
> 
> > [...]
> >
> >  double
> > -app_tminterval(int stop, int usertime)
> > +real_interval(int new)
> 
> I suggest you keep app_tminterval() as the entry point here.  As
> mentioned by the comment, non-POSIX systems need to implement the same
> interface, and indeed apps/openssl/apps_win.c in libressl-portable also
> provides app_tminterval().

Okay, retained.

> > [...]
> >
> > +   static struct timespec elapsed, now, start;
> 
> Only "start" need to be static here.
> 
> > [...]
> >
> > +double
> > +user_interval(int new)
> > +{
> > +   static struct timeval elapsed, start;
> > +   static struct rusage now;
> 
> Same here.

Fixed and fixed.

> > [...]
> >
> > -   if (usertime == 0 && !mr)
> > +   if (usertime == 0 && !mr) {
> > BIO_printf(bio_err, "You have chosen to measure elapsed time 
> > instead of user CPU time.\n");
> > +   interval_function = real_interval;
> 
> It seems a bit overengineered to use a function pointer for this.  No
> need to change this code if you just keep the app_tminterval()
> interface.

Yep.  Keeping the interface eliminates the need for changes in
speed.c and apps.h.

Also, you need  to use timersub, timespecsub, etc, so
I added that to s_time.c and apps_posix.c.

--
Scott Cheloha

Index: usr.bin/openssl/apps_posix.c
===
RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
retrieving revision 1.2
diff -u -p -r1.2 apps_posix.c
--- usr.bin/openssl/apps_posix.c13 Sep 2015 12:41:01 -  1.2
+++ usr.bin/openssl/apps_posix.c21 Nov 2017 23:48:12 -
@@ -116,31 +116,48 @@
  * Functions that need to be overridden by non-POSIX operating systems.
  */
 
-#include 
+#include 
+#include 
 
-#include 
+#include 
 
 #include "apps.h"
 
-double
-app_tminterval(int stop, int usertime)
+static double
+real_interval(int stop)
 {
-   double ret = 0;
-   struct tms rus;
-   clock_t now = times();
-   static clock_t tmstart;
-
-   if (usertime)
-   now = rus.tms_utime;
-
-   if (stop == TM_START)
-   tmstart = now;
-   else {
-   long int tck = sysconf(_SC_CLK_TCK);
-   ret = (now - tmstart) / (double) tck;
+   static struct timespec start;
+   static struct timespec elapsed, now;
+
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (stop) {
+   timespecsub(, , );
+   return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
}
+   start = now;
+   return 0.0;
+}
 
-   return (ret);
+static double
+user_interval(int stop)
+{
+   static struct timeval start;
+   static struct timeval elapsed;
+   static struct rusage now;
+
+   getrusage(RUSAGE_SELF, );
+   if (stop) {
+   timersub(_utime, , );
+   return elapsed.tv_sec + elapsed.tv_usec / 100.0;
+   }
+   start = now.ru_utime;
+   return 0.0;
+}
+
+double
+app_tminterval(int stop, int usertime)
+{
+   return (usertime) ? user_interval(stop) : real_interval(stop);
 }
 
 int
Index: usr.bin/openssl/s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.18
diff -u -p -r1.18 s_time.c
--- usr.bin/openssl/s_time.c2 Nov 2017 00:31:49 -   1.18
+++ usr.bin/openssl/s_time.c21 Nov 2017 23:48:12 -
@@ -63,11 +63,13 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -248,7 +250,7 @@ s_time_main(int argc, char **argv)
double totalTime = 0.0;
int nConn = 0;
SSL *scon = NULL;
-   time_t finishtime;
+   struct timespec finishtime, now;
int ret = 1;
char buf[1024 * 8];
int ver;
@@ -330,10 +332,12 @@ s_time_main(int argc, char **argv)
/* Loop and time how long it takes to make connections */
 
bytes_read = 0;
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
tm_Time_F(START);
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((scon = doConnection(NULL)) == NULL)
goto end;
@@ -383,7 +387,7 @@ s_time_main(int argc, char **argv)
nConn, totalTime, ((double) nConn / totalTime), bytes_read);

Re: openssl s_time, speed: use monotime for absolute interval measurement

2017-11-18 Thread Jeremie Courreges-Anglas 
On Sat, Nov 11 2017, Scott Cheloha  wrote:
> Hi,

Hi,

> times(3) is okay for user CPU measurement but is inappropriate
> for absolute interval measurement as its output is subject to
> changes by both adjtime(2) and settimeofday(2).
>
> The attached diff replaces it with getrusage(2) for user CPU
> measurement and clock_gettime(2)'s CLOCK_MONOTONIC clock for
> absolute interval measurement.
>
> The attached diff also replaces time(3) in s_time with
> clock_gettime's CLOCK_MONOTONIC clock.  This ensures that we
> only measure for about as long as the user said to.
>
> Neither timersub(2) nor timespecsub are standard, though many
> systems have them.  Is this a problem for libressl-portable?

I doubt that timersub/timespecsub are a big problem to add to -portable,
they're just macros.  clock_gettime and getrusage seem to already be
used in libressl(-portable), there's autoconf glue for the former at
least.

> Thoughts and feedback?

More comment inline.

> --
> Scott Cheloha
>
> Index: usr.bin/openssl/apps.h
> ===
> RCS file: /cvs/src/usr.bin/openssl/apps.h,v
> retrieving revision 1.19
> diff -u -p -r1.19 apps.h
> --- usr.bin/openssl/apps.h30 Aug 2016 14:34:59 -  1.19
> +++ usr.bin/openssl/apps.h10 Nov 2017 18:38:13 -
> @@ -277,9 +277,8 @@ unsigned char *next_protos_parse(unsigne
>  
>  int app_isdir(const char *);
>  
> -#define TM_START 0
> -#define TM_STOP  1
> -double app_tminterval (int stop, int usertime);
> +double real_interval(int new);
> +double user_interval(int new);
>  
>  #define OPENSSL_NO_SSL_INTERN
>  
> Index: usr.bin/openssl/apps_posix.c
> ===
> RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 apps_posix.c
> --- usr.bin/openssl/apps_posix.c  13 Sep 2015 12:41:01 -  1.2
> +++ usr.bin/openssl/apps_posix.c  10 Nov 2017 18:38:13 -
> @@ -116,31 +116,39 @@
>   * Functions that need to be overridden by non-POSIX operating systems.
>   */
>  
> -#include 
> +#include 
>  
> -#include 
> +#include 
>  
>  #include "apps.h"
>  
>  double
> -app_tminterval(int stop, int usertime)
> +real_interval(int new)

I suggest you keep app_tminterval() as the entry point here.  As
mentioned by the comment, non-POSIX systems need to implement the same
interface, and indeed apps/openssl/apps_win.c in libressl-portable also
provides app_tminterval().

>  {
> - double ret = 0;
> - struct tms rus;
> - clock_t now = times();
> - static clock_t tmstart;
> -
> - if (usertime)
> - now = rus.tms_utime;
> -
> - if (stop == TM_START)
> - tmstart = now;
> - else {
> - long int tck = sysconf(_SC_CLK_TCK);
> - ret = (now - tmstart) / (double) tck;
> + static struct timespec elapsed, now, start;

Only "start" need to be static here.

> +
> + clock_gettime(CLOCK_MONOTONIC, );
> + if (new) {
> + start = now;
> + return 0.0;
>   }
> + timespecsub(, , );
> + return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
> +}
>  
> - return (ret);
> +double
> +user_interval(int new)
> +{
> + static struct timeval elapsed, start;
> + static struct rusage now;

Same here.

> +
> + getrusage(RUSAGE_SELF, );
> + if (new) {
> + start = now.ru_utime;
> + return 0.0;
> + }
> + timersub(_utime, , );
> + return elapsed.tv_sec + elapsed.tv_usec / 100.0;
>  }
>  
>  int
> Index: usr.bin/openssl/s_time.c
> ===
> RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 s_time.c
> --- usr.bin/openssl/s_time.c  2 Nov 2017 00:31:49 -   1.18
> +++ usr.bin/openssl/s_time.c  10 Nov 2017 18:38:13 -
> @@ -68,6 +68,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -229,14 +230,9 @@ s_time_usage(void)
>  /***
>   * TIME - time functions
>   */
> -#define START0
> -#define STOP 1
> -
> -static double
> -tm_Time_F(int s)
> -{
> - return app_tminterval(s, 1);
> -}
> +#define START1
> +#define STOP 0
> +#define tm_Time_F(s) user_interval(s);
>  
>  /***
>   * MAIN - main processing area for client
> @@ -248,7 +244,7 @@ s_time_main(int argc, char **argv)
>   double totalTime = 0.0;
>   int nConn = 0;
>   SSL *scon = NULL;
> - time_t finishtime;
> + struct timespec finishtime, now;
>   int ret = 1;
>   char buf[1024 * 8];
>   int ver;
> @@ -330,10 +326,12 @@ s_time_main(int argc, char **argv)
>   /* Loop and time how long it takes to make connections */
>  
>   bytes_read = 0;
> -

openssl s_time, speed: use monotime for absolute interval measurement

2017-11-11 Thread Scott Cheloha 
Hi,

times(3) is okay for user CPU measurement but is inappropriate
for absolute interval measurement as its output is subject to
changes by both adjtime(2) and settimeofday(2).

The attached diff replaces it with getrusage(2) for user CPU
measurement and clock_gettime(2)'s CLOCK_MONOTONIC clock for
absolute interval measurement.

The attached diff also replaces time(3) in s_time with
clock_gettime's CLOCK_MONOTONIC clock.  This ensures that we
only measure for about as long as the user said to.

Neither timersub(2) nor timespecsub are standard, though many
systems have them.  Is this a problem for libressl-portable?

Thoughts and feedback?

--
Scott Cheloha

Index: usr.bin/openssl/apps.h
===
RCS file: /cvs/src/usr.bin/openssl/apps.h,v
retrieving revision 1.19
diff -u -p -r1.19 apps.h
--- usr.bin/openssl/apps.h  30 Aug 2016 14:34:59 -  1.19
+++ usr.bin/openssl/apps.h  10 Nov 2017 18:38:13 -
@@ -277,9 +277,8 @@ unsigned char *next_protos_parse(unsigne
 
 int app_isdir(const char *);
 
-#define TM_START   0
-#define TM_STOP1
-double app_tminterval (int stop, int usertime);
+double real_interval(int new);
+double user_interval(int new);
 
 #define OPENSSL_NO_SSL_INTERN
 
Index: usr.bin/openssl/apps_posix.c
===
RCS file: /cvs/src/usr.bin/openssl/apps_posix.c,v
retrieving revision 1.2
diff -u -p -r1.2 apps_posix.c
--- usr.bin/openssl/apps_posix.c13 Sep 2015 12:41:01 -  1.2
+++ usr.bin/openssl/apps_posix.c10 Nov 2017 18:38:13 -
@@ -116,31 +116,39 @@
  * Functions that need to be overridden by non-POSIX operating systems.
  */
 
-#include 
+#include 
 
-#include 
+#include 
 
 #include "apps.h"
 
 double
-app_tminterval(int stop, int usertime)
+real_interval(int new)
 {
-   double ret = 0;
-   struct tms rus;
-   clock_t now = times();
-   static clock_t tmstart;
-
-   if (usertime)
-   now = rus.tms_utime;
-
-   if (stop == TM_START)
-   tmstart = now;
-   else {
-   long int tck = sysconf(_SC_CLK_TCK);
-   ret = (now - tmstart) / (double) tck;
+   static struct timespec elapsed, now, start;
+
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (new) {
+   start = now;
+   return 0.0;
}
+   timespecsub(, , );
+   return elapsed.tv_sec + elapsed.tv_nsec / 10.0;
+}
 
-   return (ret);
+double
+user_interval(int new)
+{
+   static struct timeval elapsed, start;
+   static struct rusage now;
+
+   getrusage(RUSAGE_SELF, );
+   if (new) {
+   start = now.ru_utime;
+   return 0.0;
+   }
+   timersub(_utime, , );
+   return elapsed.tv_sec + elapsed.tv_usec / 100.0;
 }
 
 int
Index: usr.bin/openssl/s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.18
diff -u -p -r1.18 s_time.c
--- usr.bin/openssl/s_time.c2 Nov 2017 00:31:49 -   1.18
+++ usr.bin/openssl/s_time.c10 Nov 2017 18:38:13 -
@@ -68,6 +68,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -229,14 +230,9 @@ s_time_usage(void)
 /***
  * TIME - time functions
  */
-#define START  0
-#define STOP   1
-
-static double
-tm_Time_F(int s)
-{
-   return app_tminterval(s, 1);
-}
+#define START  1
+#define STOP   0
+#define tm_Time_F(s)   user_interval(s);
 
 /***
  * MAIN - main processing area for client
@@ -248,7 +244,7 @@ s_time_main(int argc, char **argv)
double totalTime = 0.0;
int nConn = 0;
SSL *scon = NULL;
-   time_t finishtime;
+   struct timespec finishtime, now;
int ret = 1;
char buf[1024 * 8];
int ver;
@@ -330,10 +326,12 @@ s_time_main(int argc, char **argv)
/* Loop and time how long it takes to make connections */
 
bytes_read = 0;
-   finishtime = time(NULL) + s_time_config.maxtime;
+   clock_gettime(CLOCK_MONOTONIC, );
+   finishtime.tv_sec += s_time_config.maxtime;
tm_Time_F(START);
for (;;) {
-   if (finishtime < time(NULL))
+   clock_gettime(CLOCK_MONOTONIC, );
+   if (timespeccmp(, , <))
break;
if ((scon = doConnection(NULL)) == NULL)
goto end;
@@ -383,7 +381,7 @@ s_time_main(int argc, char **argv)
nConn, totalTime, ((double) nConn / totalTime), bytes_read);
printf("%d connections in %lld real seconds, %ld bytes read per 
connection\n",
nConn,
-   (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+   (long long)(now.tv_sec - finishtime.tv_sec +