[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
On Sat, Jan 14, 2023 at 4:19 PM Mike Frysinger wrote:
> Rather than assume such coarse delays, re-use existing logic for
> probing the current filesystem resolution. This speeds up the
> testsuite significantly. On my system, it speeds -j1 up quite a
> lot -- by ~30%. While I didn't gather many samples to produce a
> statistically significant distribution, my runs seem to be fairly
> consistent with the values below with deviations of <1 minute.
>
> $ time make -j1
> BeforeAfter
> real 33m17.182s real 23m33.557s
> user 12m12.145s user 12m12.763s
> sys 1m52.308s sys 1m52.853s
>
> $ time make -j32
> BeforeAfter
> real 1m35.874s real 1m4.908s
> user 14m24.664s user 15m58.663s
> sys 2m9.297ssys 2m27.393s
>
> * configure.ac: Set test delays to am_cv_filesystem_timestamp_resolution.
> * t/aclocal-no-force.sh: Use slower sleep if subsecond APIs are missing.
> * t/ax/test-defs.in: Split sleep settings into separate variables.
> ---
> configure.ac | 10 +-
> t/aclocal-no-force.sh | 12
> t/ax/test-defs.in | 5 ++---
> 3 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index dcf2d95566a0..d3a67d5ffec9 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -172,15 +172,7 @@ result=no
> test "x$am_cv_prog_ln" = xln && result=yes
> AC_MSG_RESULT([$result])
>
> -# The amount we should wait after modifying files depends on the platform.
> -# On Windows '95, '98 and ME, files modifications have 2-seconds
> -# granularity and can be up to 3 seconds in the future w.r.t. the
> -# system clock. When it is important to ensure one file is older
> -# than another we wait at least 5 seconds between creations.
> -case $build in
> - *-pc-msdosdjgpp) MODIFICATION_DELAY=5;;
> - *) MODIFICATION_DELAY=2;;
> -esac
> +MODIFICATION_DELAY=$am_cv_filesystem_timestamp_resolution
> AC_SUBST([MODIFICATION_DELAY])
>
> ## --- ##
> diff --git a/t/aclocal-no-force.sh b/t/aclocal-no-force.sh
> index 3e0c04d12f18..2e139d75cf74 100644
> --- a/t/aclocal-no-force.sh
> +++ b/t/aclocal-no-force.sh
> @@ -19,6 +19,18 @@
>
> . test-init.sh
>
> +# Automake relies on high resolution timestamps in perl. If support isn't
> +# available (see lib/Automake/FileUtils.pm), then fallback to coarse sleeps.
> +# The creative quoting is to avoid spuriously triggering a failure in
> +# the maintainer checks.
> +case ${sleep_delay} in
> +0*)
> + if ! $PERL -e 'use Time::HiRes' 2>/dev/null; then
> +sleep='sleep ''2'
> + fi
> + ;;
Thanks, Mike. Thanks for this.
Do you feel like adjusting this in light of Paul's
recent change to require a version of Perl new enough to
always provide Time::HiRes support?
https://git.savannah.gnu.org/cgit/automake.git/commit/?id=01bf65daf6f6627b56fbe78fc436fd877ccd3537
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
Mike Frysinger wrote: On 14 Jan 2023 21:43, Jacob Bachmeyer wrote: [...] You could also exploit that || short-circuits in the shell and replace the "if" block with " $PERL ... || sleep='sleep ''2' ". This allows you to directly execute a command on a false result and (I think) it is portable, too. (I half-expect someone to correct me on that along the lines of "the shell on Obscurix has a bug where || implicitly uses a subshell".) personally i find (ab)use of `||` and `&&` tends to lead to unmaintainable code. i.e. it tends to produce write-once-read-never code akin to most perl. so if the construct has been in use already and isn't causing issues, i'd use it. In Perl at least, use of the "word forms" of those operators for flow control is considered idiomatic Perl, most often seen in the form "open ... or die ...;" and indeed in another of your patches as "stat ... or fatal ...". :-) -- Jacob
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
i'll note that Automake tests have been using `if ! ...`
since 1.12 (2012),
The automake *tests* intentionally use modern shell syntax and
functionality, because they go to a lot of trouble to set up an
environment where those are supported, via test-init.sh. t/README talks
about this ("prefer using POSIX constructs ..."), and much else.
Some previous maintainers made that decision.
User-level scripts, non-test configure.ac's, etc., are another matter,
and should follow the "maximum portability" rules. --thanks, karl.
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
On 14 Jan 2023 21:43, Jacob Bachmeyer wrote:
> Mike Frysinger wrote:
> > --- a/t/aclocal-no-force.sh
> > +++ b/t/aclocal-no-force.sh
> > @@ -19,6 +19,18 @@
> >
> > . test-init.sh
> >
> > +# Automake relies on high resolution timestamps in perl. If support isn't
> > +# available (see lib/Automake/FileUtils.pm), then fallback to coarse
> > sleeps.
> > +# The creative quoting is to avoid spuriously triggering a failure in
> > +# the maintainer checks.
> > +case ${sleep_delay} in
> > +0*)
> > + if ! $PERL -e 'use Time::HiRes' 2>/dev/null; then
> > +sleep='sleep ''2'
> > + fi
> > + ;;
> > +esac
> > +
>
> I seem to remember being told that "if !" is non-portable. Is there
> some other mechanism that ensures this is always run with Bash or might
> "if $PERL ... ; then :; else" be a better option for that line?
you're right:
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.71/html_node/Limitations-of-Builtins.html
i'll note that Automake tests have been using `if ! ...` since 1.12 (2012),
and no one seems to have complained. further, the shell we're using here is
the one autoconf and/or we detected, so it should avoid older broken ones.
i'm inclined to not bend over backwards for this.
> Also, you could write that Perl command as "$PERL -MTime::HiRes -e 1
> 2>/dev/null" and avoid needing any quotes there, although I suspect this
> is simply a matter of style and the comment refers to the quotes when
> setting $sleep.
yes, the quoting comment is referring to the sleep statement. i can clarify
in the comment by referring to "sleep".
> You could also exploit that || short-circuits in the shell and replace
> the "if" block with " $PERL ... || sleep='sleep ''2' ". This allows you
> to directly execute a command on a false result and (I think) it is
> portable, too. (I half-expect someone to correct me on that along the
> lines of "the shell on Obscurix has a bug where || implicitly uses a
> subshell".)
personally i find (ab)use of `||` and `&&` tends to lead to unmaintainable code.
i.e. it tends to produce write-once-read-never code akin to most perl. so if
the construct has been in use already and isn't causing issues, i'd use it.
-mike
signature.asc
Description: PGP signature
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
On 14/01/2023, Zack Weinberg wrote: > On Sat, Jan 14, 2023, at 7:18 PM, Mike Frysinger wrote: >> Rather than assume such coarse delays, re-use existing logic for >> probing the current filesystem resolution. This speeds up the >> testsuite significantly.> [...] > No objection to this patch in itself, but I want to make sure you're > aware that the "existing logic for probing the current filesystem > resolution" has a bug where, if you start running the script at just > the wrong time, it will erroneously detect a finer timestamp resolution > than the system actually supports. For instance, if we can sleep for > 0.1 second, the filesystem's timestamp resolution is 2s, and the sleep > loop happens to start executing at 0h00m59.9s, then it'll tick over to > 0h01m00.0s and conftest.file.a and conftest.file.b will have distinct > timestamps. This happens to me quite reliably: whenever I try to > run the Automake test suite inside AFS, I'll get a couple of spurious > test failures because of this bug. I don't know how exactly this delay is used in the test suite, but if the goal is just to "reliably wait until newly-created files have different timestamps from prior-created files, without waiting substantially longer than required", I've had good results with a pattern like this in my own test suites (instead of sleeping for a predetermined amount of time): cat >uptodate.mk <<'EOF' old: new false EOF touch old; touch new while make -f uptodate.mk >/dev/null 2>&1; do touch new done Cheers, Nick
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
Mike Frysinger wrote:
Rather than assume such coarse delays, re-use existing logic for
probing the current filesystem resolution. This speeds up the
testsuite significantly. On my system, it speeds -j1 up quite a
lot -- by ~30%. While I didn't gather many samples to produce a
statistically significant distribution, my runs seem to be fairly
consistent with the values below with deviations of <1 minute.
[...]
diff --git a/t/aclocal-no-force.sh b/t/aclocal-no-force.sh
index 3e0c04d12f18..2e139d75cf74 100644
--- a/t/aclocal-no-force.sh
+++ b/t/aclocal-no-force.sh
@@ -19,6 +19,18 @@
. test-init.sh
+# Automake relies on high resolution timestamps in perl. If support isn't
+# available (see lib/Automake/FileUtils.pm), then fallback to coarse sleeps.
+# The creative quoting is to avoid spuriously triggering a failure in
+# the maintainer checks.
+case ${sleep_delay} in
+0*)
+ if ! $PERL -e 'use Time::HiRes' 2>/dev/null; then
+sleep='sleep ''2'
+ fi
+ ;;
+esac
+
cat >> configure.ac << 'END'
SOME_DEFS
AC_CONFIG_FILES([sub/Makefile])
I seem to remember being told that "if !" is non-portable. Is there
some other mechanism that ensures this is always run with Bash or might
"if $PERL ... ; then :; else" be a better option for that line?
Also, you could write that Perl command as "$PERL -MTime::HiRes -e 1
2>/dev/null" and avoid needing any quotes there, although I suspect this
is simply a matter of style and the comment refers to the quotes when
setting $sleep.
You could also exploit that || short-circuits in the shell and replace
the "if" block with " $PERL ... || sleep='sleep ''2' ". This allows you
to directly execute a command on a false result and (I think) it is
portable, too. (I half-expect someone to correct me on that along the
lines of "the shell on Obscurix has a bug where || implicitly uses a
subshell".)
-- Jacob
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
On Sat, Jan 14, 2023, at 7:18 PM, Mike Frysinger wrote: > Rather than assume such coarse delays, re-use existing logic for > probing the current filesystem resolution. This speeds up the > testsuite significantly. On my system, it speeds -j1 up quite a > lot -- by ~30%. While I didn't gather many samples to produce a > statistically significant distribution, my runs seem to be fairly > consistent with the values below with deviations of <1 minute. No objection to this patch in itself, but I want to make sure you're aware that the "existing logic for probing the current filesystem resolution" has a bug where, if you start running the script at just the wrong time, it will erroneously detect a finer timestamp resolution than the system actually supports. For instance, if we can sleep for 0.1 second, the filesystem's timestamp resolution is 2s, and the sleep loop happens to start executing at 0h00m59.9s, then it'll tick over to 0h01m00.0s and conftest.file.a and conftest.file.b will have distinct timestamps. This happens to me quite reliably: whenever I try to run the Automake test suite inside AFS, I'll get a couple of spurious test failures because of this bug. zw
[bug#60807] [PATCH v2] tests: reuse am_cv_filesystem_timestamp_resolution
Rather than assume such coarse delays, re-use existing logic for
probing the current filesystem resolution. This speeds up the
testsuite significantly. On my system, it speeds -j1 up quite a
lot -- by ~30%. While I didn't gather many samples to produce a
statistically significant distribution, my runs seem to be fairly
consistent with the values below with deviations of <1 minute.
$ time make -j1
BeforeAfter
real 33m17.182s real 23m33.557s
user 12m12.145s user 12m12.763s
sys 1m52.308s sys 1m52.853s
$ time make -j32
BeforeAfter
real 1m35.874s real 1m4.908s
user 14m24.664s user 15m58.663s
sys 2m9.297ssys 2m27.393s
* configure.ac: Set test delays to am_cv_filesystem_timestamp_resolution.
* t/aclocal-no-force.sh: Use slower sleep if subsecond APIs are missing.
* t/ax/test-defs.in: Split sleep settings into separate variables.
---
configure.ac | 10 +-
t/aclocal-no-force.sh | 12
t/ax/test-defs.in | 5 ++---
3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/configure.ac b/configure.ac
index dcf2d95566a0..d3a67d5ffec9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -172,15 +172,7 @@ result=no
test "x$am_cv_prog_ln" = xln && result=yes
AC_MSG_RESULT([$result])
-# The amount we should wait after modifying files depends on the platform.
-# On Windows '95, '98 and ME, files modifications have 2-seconds
-# granularity and can be up to 3 seconds in the future w.r.t. the
-# system clock. When it is important to ensure one file is older
-# than another we wait at least 5 seconds between creations.
-case $build in
- *-pc-msdosdjgpp) MODIFICATION_DELAY=5;;
- *) MODIFICATION_DELAY=2;;
-esac
+MODIFICATION_DELAY=$am_cv_filesystem_timestamp_resolution
AC_SUBST([MODIFICATION_DELAY])
## --- ##
diff --git a/t/aclocal-no-force.sh b/t/aclocal-no-force.sh
index 3e0c04d12f18..2e139d75cf74 100644
--- a/t/aclocal-no-force.sh
+++ b/t/aclocal-no-force.sh
@@ -19,6 +19,18 @@
. test-init.sh
+# Automake relies on high resolution timestamps in perl. If support isn't
+# available (see lib/Automake/FileUtils.pm), then fallback to coarse sleeps.
+# The creative quoting is to avoid spuriously triggering a failure in
+# the maintainer checks.
+case ${sleep_delay} in
+0*)
+ if ! $PERL -e 'use Time::HiRes' 2>/dev/null; then
+sleep='sleep ''2'
+ fi
+ ;;
+esac
+
cat >> configure.ac << 'END'
SOME_DEFS
AC_CONFIG_FILES([sub/Makefile])
diff --git a/t/ax/test-defs.in b/t/ax/test-defs.in
index e09a387cd0a6..321602cfdd0b 100644
--- a/t/ax/test-defs.in
+++ b/t/ax/test-defs.in
@@ -180,9 +180,8 @@ TEX=${AM_TESTSUITE_TEX-'@TEX@'}
# The amount we should wait after modifying files depends on the platform.
# For instance, Windows '95, '98 and ME have 2-second granularity
# and can be up to 3 seconds in the future w.r.t. the system clock.
-# The creative quoting is to avoid spuriously triggering a failure in
-# the maintainer checks,
-sleep='sleep ''@MODIFICATION_DELAY@'
+sleep_delay=@MODIFICATION_DELAY@
+sleep="sleep ${sleep_delay}"
# An old timestamp that can be given to a file, in "touch -t" format.
# The time stamp should be portable to all file systems of interest.
--
2.39.0
