Re: timevar: further work

[Akim Demaille]

Hi Bruno,

I’m going to be incremental on this.

> Le 30 sept. 2018 à 01:18, Bruno Haible <address@hidden> a écrit :
> 
> Hi Akim,
> 
> - Portability: According to the Gnulib documentation, 'times' is missing on
>  native Windows (mingw, MSVC). The Gnulib module 'times' has a replacement for
>  it, that is better than the clock() fallback. How about making 'timevar'
>  depend on the Gnulib module 'times'? Then you can ditch at least the
>  USE_CLOCK code.

Here is my proposal to rely on gnulib for time issues.

commit 382f32bb5696168a80fbae5c5dc9dd6c1342e68b
Author: Akim Demaille <address@hidden>
Date:   Tue Oct 2 06:19:19 2018 +0200

    timevar: rely on gnulib modules for time portability.
    
    * modules/timevar (Depends-on): Add sys_time, sys_times, and times.
    * m4/timevar.m4: Don't check for clock_t and struct tms,
    guaranteed by gnulib.
    * lib/timevar.h: Use extern "C" protection.
    Include <stdio.h> for FILE.
    * lib/timevar.c: Include sys/time.h, sys/times.h unconditionally,
    they are guaranteed by gnulib.
    Remove uses of clock as (now useless) fallback.

diff --git a/ChangeLog b/ChangeLog
index 097f7ded4..a47845745 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2018-10-02  Akim Demaille  <address@hidden>
+
+       timevar: rely on gnulib modules for time portability.
+       * modules/timevar (Depends-on): Add sys_time, sys_times, and times.
+       * m4/timevar.m4: Don't check for clock_t and struct tms,
+       guaranteed by gnulib.
+       * lib/timevar.h: Use extern "C" protection.
+       Include <stdio.h> for FILE.
+       * lib/timevar.c: Include sys/time.h, sys/times.h unconditionally,
+       they are guaranteed by gnulib.
+       Remove uses of clock as (now useless) fallback.
+
 2018-10-01  Bruno Haible  <address@hidden>
 
        mkostemp, mkostemps: Update documentation.
diff --git a/lib/timevar.c b/lib/timevar.c
index a2d243392..391802838 100644
--- a/lib/timevar.c
+++ b/lib/timevar.c
@@ -24,38 +24,19 @@
 #include "timevar.h"
 
 #include <stdio.h>
-#include <string.h>
 #include <stdlib.h>
+#include <string.h>
+#include <sys/time.h>
+#include <sys/times.h>
 
 #include "gettext.h"
 #define _(msgid) gettext (msgid)
 #include "xalloc.h"
 
-#if HAVE_SYS_TIME_H
-# include <sys/time.h>
-#endif
-
-#ifdef HAVE_SYS_TIMES_H
-# include <sys/times.h>
-#endif
 #ifdef HAVE_SYS_RESOURCE_H
 # include <sys/resource.h>
 #endif
 
-#ifndef HAVE_CLOCK_T
-typedef int clock_t;
-#endif
-
-#ifndef HAVE_STRUCT_TMS
-struct tms
-{
-  clock_t tms_utime;
-  clock_t tms_stime;
-  clock_t tms_cutime;
-  clock_t tms_cstime;
-};
-#endif
-
 /* Calculation of scale factor to convert ticks to microseconds.
    We mustn't use CLOCKS_PER_SEC except with clock().  */
 #if HAVE_SYSCONF && defined _SC_CLK_TCK
@@ -79,17 +60,12 @@ struct tms
 # define USE_GETRUSAGE
 # define HAVE_USER_TIME
 # define HAVE_SYS_TIME
-#elif defined HAVE_CLOCK
-# define USE_CLOCK
-# define HAVE_USER_TIME
 #endif
 
 #if defined USE_TIMES && !defined HAVE_DECL_TIMES
 clock_t times (struct tms *);
 #elif defined USE_GETRUSAGE && !defined HAVE_DECL_GETRUSAGE
 int getrusage (int, struct rusage *);
-#elif defined USE_CLOCK && !defined HAVE_DECL_CLOCK
-clock_t clock (void);
 #endif
 
 /* libc is very likely to have snuck a call to sysconf() into one of
@@ -99,9 +75,6 @@ clock_t clock (void);
 #ifdef USE_TIMES
 static float ticks_to_msec;
 # define TICKS_TO_MSEC (1.0 / TICKS_PER_SECOND)
-#elif defined USE_CLOCK
-static float clocks_to_msec;
-# define CLOCKS_TO_MSEC (1.0 / CLOCKS_PER_SEC)
 #endif
 
 /* See timevar.h for an explanation of timing variables.  */
@@ -184,8 +157,6 @@ set_to_current_time (struct timevar_time_def *now)
     getrusage (RUSAGE_CHILDREN, &rusage);
     now->user = rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec * 1e-6;
     now->sys  = rusage.ru_stime.tv_sec + rusage.ru_stime.tv_usec * 1e-6;
-#elif defined USE_CLOCK
-    now->user = clock () * clocks_to_msec;
 #endif
   }
 }
@@ -229,8 +200,6 @@ timevar_init ()
 
 #if defined USE_TIMES
   ticks_to_msec = TICKS_TO_MSEC;
-#elif defined USE_CLOCK
-  clocks_to_msec = CLOCKS_TO_MSEC;
 #endif
 }
 
diff --git a/lib/timevar.h b/lib/timevar.h
index 88c4aa864..7c9258409 100644
--- a/lib/timevar.h
+++ b/lib/timevar.h
@@ -19,7 +19,13 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #ifndef _TIMEVAR_H
-#define _TIMEVAR_H
+# define _TIMEVAR_H 1
+
+# include <stdio.h>
+
+# ifdef  __cplusplus
+extern "C" {
+# endif
 
 /* Timing variables are used to measure elapsed time in various
    portions of the application.  Each measures elapsed user, system, and
@@ -121,4 +127,8 @@ void timevar_print (FILE *fp);
 /* Set to to nonzero to enable timing variables.  */
 extern int timevar_enabled;
 
+# ifdef  __cplusplus
+}
+# endif
+
 #endif /* ! _TIMEVAR_H */
diff --git a/m4/timevar.m4 b/m4/timevar.m4
index b41841342..c790ed0a9 100644
--- a/m4/timevar.m4
+++ b/m4/timevar.m4
@@ -17,11 +17,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# serial 2
+# serial 3
 
 AC_DEFUN([gl_TIMEVAR],
 [AC_CHECK_HEADERS([sys/time.h sys/times.h])
- AC_CHECK_HEADERS([sys/resource.h],,,
+ AC_CHECK_HEADERS([sys/resource.h], [], [],
    [$ac_includes_default
 #if HAVE_SYS_TIME_H
 # include <sys/time.h>
@@ -44,17 +44,4 @@ AC_CHECK_DECLS([getrusage, times, clock, sysconf], [], [],
 # include <sys/resource.h>
 #endif
 ])
-
-AC_CHECK_TYPES([clock_t, struct tms], [], [],
-[$ac_includes_default
-#if HAVE_SYS_TIME_H
-# include <sys/time.h>
-#endif
-#if HAVE_SYS_TIMES_H
-# include <sys/times.h>
-#endif
-#if HAVE_SYS_RESOURCE_H
-# include <sys/resource.h>
-#endif
-])
 ])
diff --git a/modules/timevar b/modules/timevar
index fdbbd6b79..05dac7609 100644
--- a/modules/timevar
+++ b/modules/timevar
@@ -8,8 +8,11 @@ m4/timevar.m4
 
 Depends-on:
 gettext-h
-xalloc
 stdlib
+sys_time
+sys_times
+times
+xalloc
 
 configure.ac:
 gl_TIMEVAR


> - Is the 'times' approach preferrable to the 'getrusage' approach in all
>  cases? I’m thinking of multithreading and of processes that fork() children.

On this machine, man times gives:

> SYNOPSIS
>      #include <sys/times.h>
> 
>      clock_t
>      times(struct tms *tp);
> 
> DESCRIPTION
>      This interface is obsoleted by getrusage(2) and gettimeofday(2).

So I guess in all cases I should prefer getrusage to times.
Yet getrusage does not give me the wall clock time, so I
suppose I should use getrusage + time (not times).

https://stackoverflow.com/a/12480485/1353549