[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 1/2] mtime: use Time::HiRes::stat when available for subsecond resolution
Mike Frysinger wrote:
Perl's builtin stat function returns timestamps that have 1 second
resolution. This can lead automake needlessly regenerating files
because it compares timestamps as "older than or equal to" rather
than only "older than". This is perfectly reasonable as we have
no way of knowing what file is older if they have the same value
which means we must be pessimistic & assume an upate is required.
However, given modern systems that are quite fast and can easily
generate many files in less than an second, we end up doing a lot
of extra work.
Until Perl gets around to figuring out how to support subsecond
timestamp resolution, optionally import the Time::HiRes module and
use its stat function instead. If it's available, great, if not,
then we're no worse off than we are today.
Performance-wise, at least by using the testsuite, there doesn't
seem to be any measurable difference.
* lib/Automake/FileUtils.pm: Use Time::HiRes to lookup mtimes on
files if available.
---
lib/Automake/FileUtils.pm | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/lib/Automake/FileUtils.pm b/lib/Automake/FileUtils.pm
index 848ff22d1761..78e0942e9f53 100644
--- a/lib/Automake/FileUtils.pm
+++ b/lib/Automake/FileUtils.pm
@@ -42,6 +42,11 @@ use Exporter;
use File::stat;
use IO::File;
+# Perl's builtin stat does not provide sub-second resolution. Use Time::HiRes
+# if it's available instead. Hopefully one day perl will update.
+# https://github.com/Perl/perl5/issues/17900
+my $have_time_hires = eval { require Time::HiRes; };
+
use Automake::Channels;
use Automake::ChannelDefs;
@@ -115,10 +120,18 @@ sub mtime ($)
return 0
if $file eq '-' || ! -f $file;
- my $stat = stat ($file)
-or fatal "cannot stat $file: $!";
-
- return $stat->mtime;
+ if ($have_time_hires)
+{
+ my @stat = Time::HiRes::stat ($file)
+ or fatal "cannot stat $file: $!";
+ return $stat[9];
+}
+ else
+{
+ my $stat = stat ($file)
+ or fatal "cannot stat $file: $!";
+ return $stat->mtime;
+}
}
If you change that variable to a constant, you can eliminate the runtime
overhead entirely, since Perl optimizes if(1) and if(0) and folds
constants at compile time.
Something like:
use constant HAVE_Time_HiRes => eval { require Time::HiRes; };
Then:
if (HAVE_Time_HiRes)
...
If you do this, Perl will inline the block actually used and elide the
branch at runtime. This is generally useful for any test that can only
go one way in a specific run of the program.
-- 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#60776] [PATCH] distdir/emacs: avoid `test -d` with MKDIR_P
On 14 Jan 2023 17:05, Karl Berry wrote: > most of the code directly preceding & following this line use @. > i.e. the vast majority of the current distdir logic. > > Yeah. > > If you feel like changing those @'s to AM_v_at while we're here, sounds > good to me ... -k i'd like to give it a bit more thought as to what the output should look like in general in a non-verbose build, so i'll do it in a follow up -mike signature.asc Description: PGP signature
[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
[bug#60776] [PATCH] distdir/emacs: avoid `test -d` with MKDIR_P
most of the code directly preceding & following this line use @. i.e. the vast majority of the current distdir logic. Yeah. If you feel like changing those @'s to AM_v_at while we're here, sounds good to me ... -k
[bug#60807] [PATCH 1/2] mtime: use Time::HiRes::stat when available for subsecond resolution
On 14 Jan 2023 14:52, Karl Berry wrote:
> +my $have_time_hires = eval { require Time::HiRes; };
>
> I don't object. Although if there's no speed up in practice, I wonder if
> it's worth the extra code (simple-enough though it is). -k
there's no speed up in the execution of a single process, but there is a
speedup for users when using these tools back-to-back as they don't have
to regen as often. this fix is necessary for the next patch to speed up
the testsuite by using a smaller delay.
although that makes me realize i need to make the next patch work when
the system doesn't have Time::HiRes so users don't hit test failures.
there was only one test that checks current behavior, so shouldn't be
too bad.
-mike
signature.asc
Description: PGP signature
[bug#60776] [PATCH] distdir/emacs: avoid `test -d` with MKDIR_P
On 14 Jan 2023 15:17, Karl Berry wrote: > imo we overly rely on explicit @ in many places which can make debugging > failures painful. > > FWIW, I agree. Although I see no @ here. most of the code directly preceding & following this line use @. i.e. the vast majority of the current distdir logic. -mike signature.asc Description: PGP signature
[bug#60776] [PATCH] distdir/emacs: avoid `test -d` with MKDIR_P
imo we overly rely on explicit @ in many places which can make debugging failures painful. FWIW, I agree. Although I see no @ here. if we ever get to that day, we'd use $(AM_V_at) in this location. Ok by me. --thanks, karl.
[bug#60807] [PATCH 1/2] mtime: use Time::HiRes::stat when available for subsecond resolution
+my $have_time_hires = eval { require Time::HiRes; };
I don't object. Although if there's no speed up in practice, I wonder if
it's worth the extra code (simple-enough though it is). -k
[bug#60808] [PATCH 2/2] tests: reuse am_cv_filesystem_timestamp_resolution
-case $build in - *-pc-msdosdjgpp) MODIFICATION_DELAY=5;; - *) MODIFICATION_DELAY=2;; -esac +MODIFICATION_DELAY=$am_cv_filesystem_timestamp_resolution Looks fantastic to me :). --thanks, karl.
[bug#59994] [PATCH] tests: Don't try to prevent flex to include unistd.h
The *command line option* --never-interactive was added in version 2.5.6 (along with long command line options in general). That version was released somewhere between 2002-04-19 and 2002-04-23; I guess 20 years ago is long enough, barely :). Though if it's just as easy to use the %option, why not ... since we already have to assume flex (specifically) either way ... --thanks, karl.
[bug#59994] [PATCH] tests: Don't try to prevent flex to include unistd.h
On Fri, Jan 13, 2023, at 6:33 PM, Karl Berry wrote:
>> your patch *and* consistently test flex with "--never-interactive".
>
> Making flex non-interactive sounds desirable in any case, but
> --never-interactive is not mentioned in the 2.6.0 --help message.
> Instead there is -B (--batch), although --never-interactive is
> recognized, I can see experimentally.
--never-interactive is documented in the Texinfo manual ("Options
Affecting Scanner Behavior" node). It looks to me like the --help
output intentionally lists only the most commonly used options.
--never-interactive and --batch are technically orthogonal. I don't
know any reason a *real* scanner would want to use --never-interactive
*without* --batch. However, to address the specific problem with *test*
scanners we are discussing here, --never-interactive is what we want,
because --never-interactive is the option that makes the generated code
not call isatty().
> I do not know when --never-interactive, or any of its variants, was
> implemented. We should check that before using it ...
I'm pleasantly surprised to report that the Git repo for flex at
has history going back all the way
to 1987.
The *feature* of optionally not calling isatty() on the input file
has been present since version 2.5.1 (released 1995-03-28), in the
form of `%option {always,never}-interactive` directives inside the
scanner. The *command line option* --never-interactive was added
in version 2.5.6 (along with long command line options in general).
That version was released somewhere between 2002-04-19 and 2002-04-23;
the exact date is not recorded anywhere I can find.
zw
[bug#60807] [PATCH 1/2] mtime: use Time::HiRes::stat when available for subsecond resolution
Perl's builtin stat function returns timestamps that have 1 second
resolution. This can lead automake needlessly regenerating files
because it compares timestamps as "older than or equal to" rather
than only "older than". This is perfectly reasonable as we have
no way of knowing what file is older if they have the same value
which means we must be pessimistic & assume an upate is required.
However, given modern systems that are quite fast and can easily
generate many files in less than an second, we end up doing a lot
of extra work.
Until Perl gets around to figuring out how to support subsecond
timestamp resolution, optionally import the Time::HiRes module and
use its stat function instead. If it's available, great, if not,
then we're no worse off than we are today.
Performance-wise, at least by using the testsuite, there doesn't
seem to be any measurable difference.
* lib/Automake/FileUtils.pm: Use Time::HiRes to lookup mtimes on
files if available.
---
lib/Automake/FileUtils.pm | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/lib/Automake/FileUtils.pm b/lib/Automake/FileUtils.pm
index 848ff22d1761..78e0942e9f53 100644
--- a/lib/Automake/FileUtils.pm
+++ b/lib/Automake/FileUtils.pm
@@ -42,6 +42,11 @@ use Exporter;
use File::stat;
use IO::File;
+# Perl's builtin stat does not provide sub-second resolution. Use Time::HiRes
+# if it's available instead. Hopefully one day perl will update.
+# https://github.com/Perl/perl5/issues/17900
+my $have_time_hires = eval { require Time::HiRes; };
+
use Automake::Channels;
use Automake::ChannelDefs;
@@ -115,10 +120,18 @@ sub mtime ($)
return 0
if $file eq '-' || ! -f $file;
- my $stat = stat ($file)
-or fatal "cannot stat $file: $!";
-
- return $stat->mtime;
+ if ($have_time_hires)
+{
+ my @stat = Time::HiRes::stat ($file)
+ or fatal "cannot stat $file: $!";
+ return $stat[9];
+}
+ else
+{
+ my $stat = stat ($file)
+ or fatal "cannot stat $file: $!";
+ return $stat->mtime;
+}
}
--
2.39.0
[bug#60808] [PATCH 2/2] 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. --- configure.ac | 10 +- 1 file changed, 1 insertion(+), 9 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]) ## --- ## -- 2.39.0
