On 05/04/2025 12:03, Pádraig Brady wrote:
On 05/04/2025 09:03, Paul Eggert wrote:
On 2025-04-04 12:27, Pádraig Brady wrote:
+ /* Clamp underflow to 1ns, as 0 disables the timeout. */
+ if (duration == 0 && errno == ERANGE)
+ duration = 1e-9;
That isn't exactly right as the 1e-9 double-rounds to 2e-9 when we
compute the struct timespec. Also, even with the patch the code
mishandles 16777216.000000001 (2**24 + 10**-9) by treating it as if it
were just 16777216. These are tiny bugs, I know, but I took up the
challenge of doing this things more correctly by installing the attached.
This is the first time coreutils is linking with libm.
So practically that does add measurable overhead:
$ time seq 10000 | xargs -n1 src/timeout-old >/dev/null
real 0m7.139s
$ time seq 10000 | xargs -n1 src/timeout >/dev/null
real 0m8.161s
So on my 2.6GHz i7-5600U we're adding 102,200 ns to the startup time,
to address theoretical single nanosecond issues.
Given the code is also more complex, I'm thinking using libm is not worth it?
BTW I'll apply the attached cleanups if we keep the libm dependency.
I'm going to apply the attached later to remove the libm dependency.
It's just too much of a dependency for minimal functionality.
All existing tests are maintained, and still pass.
cheers,
Pádraig
From 7b05df48a59963c31011b1891b63a99f9a89bbf1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 5 Apr 2025 21:48:05 +0100
Subject: [PATCH] timeout: remove dependence on libm
This was seen to add about 100,000 ns to the startup time,
on a 2.6 GHz i7-5600U with glibc 2.40.
* bootstrap.conf: Remove fenv-rounding and signbit deps.
* src/local.mk: Remove fenv lib dependency.
* src/timeout.c (is_negative): A new helper function to
be equivalent of signbit in the underflow case.
(parse_duration): Remove the rounding up logic,
as a nanosecond here or there has no significance.
---
bootstrap.conf | 2 --
src/local.mk | 3 ---
src/timeout.c | 36 +++++++++++-------------------------
3 files changed, 11 insertions(+), 30 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 3c133ef71..c99838e95 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -99,7 +99,6 @@ gnulib_modules="
fdatasync
fdopen
fdutimensat
- fenv-rounding
file-has-acl
file-type
fileblocks
@@ -243,7 +242,6 @@ gnulib_modules="
settime
sig2str
sigaction
- signbit
skipchars
smack
ssize_t
diff --git a/src/local.mk b/src/local.mk
index 0a4e3af43..fd9dc81c2 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -257,9 +257,6 @@ src_stat_LDADD += $(LIB_SELINUX)
# for nvlist_lookup_uint64_array
src_stat_LDADD += $(LIB_NVPAIR)
-# for fegetround, fesetround
-src_timeout_LDADD += $(FENV_ROUNDING_LIBM)
-
# for gettime, settime, tempname, utimecmp, utimens
copy_ldadd += $(CLOCK_TIME_LIB)
src_date_LDADD += $(CLOCK_TIME_LIB)
diff --git a/src/timeout.c b/src/timeout.c
index 5947c1942..fe8eb4559 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -45,9 +45,8 @@
Written by Pádraig Brady. */
#include <config.h>
-#include <fenv.h>
+#include <ctype.h>
#include <getopt.h>
-#include <math.h>
#include <stdio.h>
#include <sys/types.h>
#include <signal.h>
@@ -354,32 +353,25 @@ apply_time_suffix (double *x, char suffix_char)
return true;
}
+ATTRIBUTE_PURE static bool
+is_negative (char const *num)
+{
+ while (*num && isspace (to_uchar (*num)))
+ num++;
+ return *num == '-';
+}
+
static double
parse_duration (char const *str)
{
- /* If possible tell strtod to round up, so that we always wait at
- least as long as STR specifies. Although Standard C requires
- FENV_ACCESS ON, don't bother if using GCC as it warns. */
-#ifdef FE_UPWARD
-# if !defined __GNUC__ && 199901 <= __STDC_VERSION__
-# pragma STDC FENV_ACCESS ON
-# endif
- int round = fegetround ();
- fesetround (FE_UPWARD);
-#endif
-
char *ep;
double duration = cl_strtod (str, &ep);
-#ifdef FE_UPWARD
- fesetround (round);
-#endif
-
if (ep == str
/* Nonnegative interval. */
|| ! (0 <= duration)
/* The interval did not underflow to -0. */
- || (signbit (duration) && errno == ERANGE)
+ || (errno == ERANGE && is_negative (str))
/* No extra chars after the number and an optional s,m,h,d char. */
|| (*ep && *(ep + 1))
/* Check any suffix char and update timeout based on the suffix. */
@@ -391,16 +383,10 @@ parse_duration (char const *str)
/* Do not let the duration underflow to 0, as 0 disables the timeout.
Use 2**-30 instead of 0; settimeout will round it up to 1 ns,
- whereas 1e-9 might double-round to 2 ns.
+ whereas 1e-9 might double-round to 2 ns. */
- If FE_UPWARD is defined, this code is not needed on glibc as its
- strtod rounds upward correctly. Although this code might not be
- needed on non-glibc platforms too, it's too much trouble to
- worry about that. */
-#if !defined FE_UPWARD || !defined __GLIBC__
if (duration == 0 && errno == ERANGE)
duration = 9.313225746154785e-10;
-#endif
return duration;
}
--
2.48.1
