GNU bug report logs - #32902
Add support for (TIMESTAMP . RESOLUTION) Lisp timestamps

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 32902 in the body.
You can then email your comments to 32902 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: GNU Emacs bug reports <bug-gnu-emacs <at> gnu.org>
Subject: Add support for (TIMESTAMP . RESOLUTION) Lisp timestamps
Date: Mon, 1 Oct 2018 18:00:26 -0700

[Message part 1 (text/plain, inline)]

The attached patches follow up on a suggestion by Stefan a few weeks 
ago, by adding support for a new Lisp timestamp format (TIMESTAMP . 
FREQUENCY), where TIMESTAMP is an integer that counts clock ticks and 
FREQUENCY is a positive integer that counts ticks per second. For 
brevity the documentation says (TICKS . HZ) instead of (TIMESTAMP . 
FREQUENCY).

Although current-time and similar functions continue to return the (HI 
LO US PS) format, the idea is that Emacs eventually should switch to 
(TICKS . HZ) as it is a better match for an Emacs with bignums (among 
other things, it does not lose information), and in the meantime we can 
better document that the Lisp timestamp format has changed in the past 
and can change in the future, and that user code should not depend on 
the exact timestamp format.

[0001-Move-timestamp-related-stuff-to-systime.c.patch (text/x-patch, attachment)]

[0002-Coalesce-duplicate-make_lisp_timeval-etc.patch (text/x-patch, attachment)]

[0003-Export-converting-mpz-to-u-intmax.patch (text/x-patch, attachment)]

[0004-New-TICKS-.-HZ-timestamp-format.patch (text/x-patch, attachment)]

Message #10 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Tue, 02 Oct 2018 06:04:07 +0300

> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Mon, 1 Oct 2018 18:00:26 -0700
> 
> The attached patches follow up on a suggestion by Stefan a few weeks 
> ago, by adding support for a new Lisp timestamp format (TIMESTAMP . 
> FREQUENCY), where TIMESTAMP is an integer that counts clock ticks and 
> FREQUENCY is a positive integer that counts ticks per second. For 
> brevity the documentation says (TICKS . HZ) instead of (TIMESTAMP . 
> FREQUENCY).

Thanks for working on this.

I'd prefer not to move stuff from editfns.c to a new file, as that
makes forensics significantly harder.  Is it feasible to leave the
time-related code in editfns.c?

>  DEFUN ("current-time", Fcurrent_time, Scurrent_time, 0, 0, 0,
> -       doc: /* Return the current time, as the number of seconds since 1970-01-01 00:00:00.
> -The time is returned as a list of integers (HIGH LOW USEC PSEC).
> -HIGH has the most significant bits of the seconds, while LOW has the
> -least significant 16 bits.  USEC and PSEC are the microsecond and
> -picosecond counts.  */)
> +       doc: /* Return the current time, counting the number of seconds since the epoch.
> +
> +See Info node `(elisp)Time of Day' for the format of the returned
> +timestamp.  Although this is currently list format, it may change in
> +future versions of Emacs.  Use `encode-time' if you need a particular
> +form; for example, (encode-time nil \\='list) returns the current time
> +in list form.  */)

I think this obfuscation of the time values in doc strings (here and
elsewhere in the patch) is not a good idea, and asking users to read
the manual just to understand what kind of data they will get or need
to pass to a function, is a step backwards.  Doc strings only send to
the manuals for additional details and explanations, not for the basic
facts such as these.

The manual and NEWS can (and probably should) explain that this stuff
is in transition, but we shouldn't describe return values as opaque
objects because of that.  IMO, this is not the Emacsy way of dealing
with complex data structures.

I also don't like saying in a doc string that something might change
in a future version: that's stuff for the manual.  Doc strings should
describe the current state of affairs.

> +If TIME is a list (SECOND MINUTE HOUR DAY MONTH YEAR IGNORED DST ZONE)
> +it a decoded time in the style of `decode-time', so that (encode-time
   ^^
A typo: should be "it is" or "it's".

> +If FORM is a positive integer, the time is returned as a pair of
> +integers (TICKS . FORM), where TICKS is the number of clock ticks and FORM
> +is the clock frequency in ticks per second.  (Currently the positive
> +integer should be at least 65536 if the returned value is expected to
> +be given to standard functions expecting Lisp timestamps.)  If FORM is
> +t, the time is returned as (TICKS . PHZ), where PHZ is a
> +platform-dependent clock frequency.

This doesn't tell in what units the clock frequency is returned.

> -usage: (encode-time SECOND MINUTE HOUR DAY MONTH YEAR &optional ZONE)  */)
> +usage: (encode-time TIME &optional FORM)  */)

This makes an impression the function doesn't support more than 2
arguments, which is incorrect.  Can we provide a more accurate 'usage'
form?

I think it would be good to add tests for the functions being
modified, otherwise we might be breaking something without paying
attention.

Message #13 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 32902 <at> debbugs.gnu.org
Subject: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp timestamps
Date: Wed, 3 Oct 2018 11:45:12 -0700

[Message part 1 (text/plain, inline)]

On 10/1/18 8:04 PM, Eli Zaretskii wrote:

> Doc strings only send to
> the manuals for additional details and explanations, not for the basic
> facts such as these.

The attached proposed additional patch fixes this by referring to 
format-time-string, and by adding the description of time values to 
format-time-string's doc string. (This is better than the old practice 
of referring to current-time-string, which does not deal with subsecond 
info.) It also fixes some minor doc nits I noticed in additional reading.


> This makes an impression the function doesn't support more than 2
> arguments, which is incorrect.  Can we provide a more accurate 'usage' form?

Yes, and I gave that a shot in the attached patch.


> I think it would be good to add tests for the functions being
> modified, otherwise we might be breaking something without paying
> attention.

Good point, and done in the attached.


> Is it feasible to leave the time-related code in editfns.c?

It's feasible, and I could prepare a patch along those lines. However, 
the time code has has nothing to do with edit functions and is growing 
significantly: the proposed src/systime.c is 56 KiB, which is above the 
median size for src/*.c files. Also, this patch changes the time code so 
much that moving it to a new file won't be that much more of a forensics 
annoyance than leaving it in editfns.c. To my mind the clarity in having 
the time code in its own module outweighs the forensics annoyance.

[0001-Improvements-on-TICKS-.-HZ.patch (text/x-patch, attachment)]

Message #16 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Thu, 04 Oct 2018 19:08:27 +0300

> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Cc: 32902 <at> debbugs.gnu.org
> Date: Wed, 3 Oct 2018 11:45:12 -0700
> 
> > Doc strings only send to
> > the manuals for additional details and explanations, not for the basic
> > facts such as these.
> 
> The attached proposed additional patch fixes this by referring to 
> format-time-string, and by adding the description of time values to 
> format-time-string's doc string. (This is better than the old practice 
> of referring to current-time-string, which does not deal with subsecond 
> info.) It also fixes some minor doc nits I noticed in additional reading.
> 
> > This makes an impression the function doesn't support more than 2
> > arguments, which is incorrect.  Can we provide a more accurate 'usage' form?
> 
> Yes, and I gave that a shot in the attached patch.
> 
> > I think it would be good to add tests for the functions being
> > modified, otherwise we might be breaking something without paying
> > attention.
> 
> Good point, and done in the attached.

Thanks.

> > Is it feasible to leave the time-related code in editfns.c?
> 
> It's feasible, and I could prepare a patch along those lines. However, 
> the time code has has nothing to do with edit functions and is growing 
> significantly: the proposed src/systime.c is 56 KiB, which is above the 
> median size for src/*.c files. Also, this patch changes the time code so 
> much that moving it to a new file won't be that much more of a forensics 
> annoyance than leaving it in editfns.c. To my mind the clarity in having 
> the time code in its own module outweighs the forensics annoyance.

OK, but can we call the new file something like timefns.c?  systime.c
sounds wrong to me (these isn't system-dependent stuff).

Message #21 received at 32902-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 32902-done <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Sat, 6 Oct 2018 23:32:00 -0700

Eli Zaretskii wrote:
> OK, but can we call the new file something like timefns.c?

Sure, that's easy enough. I revamped the patches to do that and installed them 
into master. Closing the bug report.

Message #24 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Sun, 07 Oct 2018 20:55:54 +0300

> Cc: 32902-done <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sat, 6 Oct 2018 23:32:00 -0700
> 
> Eli Zaretskii wrote:
> > OK, but can we call the new file something like timefns.c?
> 
> Sure, that's easy enough. I revamped the patches to do that and installed them 
> into master. Closing the bug report.

Thanks.

I saw a compilation problem with mingw.org's MinGW:

  timefns.c: In function 'lisp_to_timespec':
  timefns.c:899:21: warning: passing argument 2 of 'mpz_time' from incompatible pointer type [-Wincompatible-pointer-types]
     if (mpz_time (*q, &result.tv_sec))
		       ^
  timefns.c:828:1: note: expected 'time_t * {aka long int *}' but argument is of type '__time64_t * {aka long long int *}'
   mpz_time (mpz_t const z, time_t *t)
   ^~~~~~~~

I fixed it, but please take a look, perhaps there's a better fix for
this situation.

Message #27 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Sun, 7 Oct 2018 13:05:58 -0700

Eli Zaretskii wrote:
>    timefns.c: In function 'lisp_to_timespec':
>    timefns.c:899:21: warning: passing argument 2 of 'mpz_time' from incompatible pointer type [-Wincompatible-pointer-types]
>       if (mpz_time (*q, &result.tv_sec))
> 		       ^
>    timefns.c:828:1: note: expected 'time_t * {aka long int *}' but argument is of type '__time64_t * {aka long long int *}'
>     mpz_time (mpz_t const z, time_t *t)
>     ^~~~~~~~
> 
> I fixed it, but please take a look, perhaps there's a better fix for
> this situation.

That's only a very partial fix, unfortunately. I looked into the matter briefly, 
and was dismayed by how much work would be needed for a real fix, even if I 
fixed only timefns.c.

Wouldn't it be much better to remove the "#define _USE_32BIT_TIME_T" from 
nt/inc/mingw_time.h? That is what's causing the problem that your fix attempted 
to paper over. A lot of code assumes that a struct timespec (and struct 
timeval's) tv_sec component is of type time_t, and defining _USE_32BIT_TIME_T 
violates that assumption; furthermore it means that MinGW Emacs stops working in 
2038 (and doesn't work even now for timestamps more than 20 years into the 
future, something that is pretty routine for me and I imagine for other users).

Whatever backward-compatibility mess that defining _USE_32BIT_TIME_T attempts to 
work around, really needs to be worked around in a better way.

Message #30 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Mon, 08 Oct 2018 05:44:30 +0300

> Cc: 32902 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sun, 7 Oct 2018 13:05:58 -0700
> 
> > I fixed it, but please take a look, perhaps there's a better fix for
> > this situation.
> 
> That's only a very partial fix, unfortunately. I looked into the matter briefly, 
> and was dismayed by how much work would be needed for a real fix, even if I 
> fixed only timefns.c.

Can you point out a couple of such places?  I'm not sure I understand
the nature of the problem.  The data width difference notwithstanding,
the values should all fit in 32-bit time_t.

> Wouldn't it be much better to remove the "#define _USE_32BIT_TIME_T" from 
> nt/inc/mingw_time.h? That is what's causing the problem that your fix attempted 
> to paper over.

I will take a closer look, but I'm not sure we can do anything about
that.  Microsoft made an incompatible change in its runtime libraries
around Windows Vista, and switched to 64-bit time_t even on 32-bit
systems.  Since we still try to support older Windows versions, we
must use that kludge, and we must limit ourselves to 32-bit time_t in
32-bit builds.

> A lot of code assumes that a struct timespec (and struct 
> timeval's) tv_sec component is of type time_t, and defining _USE_32BIT_TIME_T 
> violates that assumption; furthermore it means that MinGW Emacs stops working in 
> 2038 (and doesn't work even now for timestamps more than 20 years into the 
> future, something that is pretty routine for me and I imagine for other users).
> 
> Whatever backward-compatibility mess that defining _USE_32BIT_TIME_T attempts to 
> work around, really needs to be worked around in a better way.

The only way is to drop support for older Windows systems.  Which we
will have to do before the year 2038 (unless some other solution could
be found, which I very much doubt).

Message #33 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Sun, 7 Oct 2018 22:18:12 -0700

Eli Zaretskii wrote:

>> That's only a very partial fix, unfortunately. I looked into the matter briefly,
>> and was dismayed by how much work would be needed for a real fix, even if I
>> fixed only timefns.c.
> 
> Can you point out a couple of such places?

Well, for starters there are multiple instances of "time_t s = t.tv_sec;" where 
T is of time struct timespec, and evidently this assigns a 64-bit quantity to a 
32-bit value on the platform in question. There's lots more code like that.

> The data width difference notwithstanding,
> the values should all fit in 32-bit time_t.

Ah, I didn't know that. In that case, the changes I was thinking of might not be 
needed for timestamps before 2038. (I write "might" because I would rather not 
spend my limited time to think this through.)
> Microsoft made an incompatible change in its runtime libraries
> around Windows Vista, and switched to 64-bit time_t even on 32-bit
> systems.  Since we still try to support older Windows versions, we
> must use that kludge, and we must limit ourselves to 32-bit time_t in
> 32-bit builds.

Here are some possible suggestions:

1. Redefine 'struct timespec' and 'clock_gettime' on 32-bit MinGW so that they 
use 32-bit time_t only. The redefinitions would be visible only within Emacs; 
you wouldn't actually change MinGW.

2. Have Emacs w32*.c detect the width of the MS-Windows API's time_t at runtime, 
and if necessary convert between any 32-bit time_t on the MS-Windows side and 
the 64-bit time_t visible to the rest of the Emacs C code.

3. Build one Emacs executable for 32-bit MS-Windows Vista and later (with 64-bit 
time_t), and another one for 32-bit MS-Windows XP and older (with 32-bit time_t).

Any of these would insulate the rest of Emacs from this glitch. You mentioned a 
fourth possibility that would also serve:

> drop support for older Windows systems.

Microsoft itself has dropped support for the older MS-Windows systems in 
question, and it would be fine if Emacs dropped support too. We routinely drop 
support for obsolete and no-longer-maintained operating system versions like 
RHEL 5 and Irix 6.5.

Message #36 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Tue, 09 Oct 2018 18:09:31 +0300

> Cc: 32902 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sun, 7 Oct 2018 22:18:12 -0700
> 
> > Microsoft made an incompatible change in its runtime libraries
> > around Windows Vista, and switched to 64-bit time_t even on 32-bit
> > systems.  Since we still try to support older Windows versions, we
> > must use that kludge, and we must limit ourselves to 32-bit time_t in
> > 32-bit builds.
> 
> Here are some possible suggestions:
> 
> 1. Redefine 'struct timespec' and 'clock_gettime' on 32-bit MinGW so that they 
> use 32-bit time_t only. The redefinitions would be visible only within Emacs; 
> you wouldn't actually change MinGW.
> 
> 2. Have Emacs w32*.c detect the width of the MS-Windows API's time_t at runtime, 
> and if necessary convert between any 32-bit time_t on the MS-Windows side and 
> the 64-bit time_t visible to the rest of the Emacs C code.
> 
> 3. Build one Emacs executable for 32-bit MS-Windows Vista and later (with 64-bit 
> time_t), and another one for 32-bit MS-Windows XP and older (with 32-bit time_t).
> 
> Any of these would insulate the rest of Emacs from this glitch.

The last two are undesirable, since it is generally expected of a
single Windows binary to run on all supported systems; having 2
separate binaries is possible, but complicates the matters.

I will try to look into the first alternative, not sure if its
feasible.

> > drop support for older Windows systems.
> 
> Microsoft itself has dropped support for the older MS-Windows systems in 
> question, and it would be fine if Emacs dropped support too. We routinely drop 
> support for obsolete and no-longer-maintained operating system versions like 
> RHEL 5 and Irix 6.5.

I don't think we should follow Microsoft in their decisions.  Last
time this came up, we decided not to drop support even for Windows 9X,
and here we are talking about XP and older.  We still have a few years
to make that decision.

Message #39 received at 32902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32902 <at> debbugs.gnu.org
Subject: Re: bug#32902: Add support for (TIMESTAMP . RESOLUTION) Lisp
 timestamps
Date: Wed, 10 Oct 2018 06:49:03 +0300

> Cc: 32902 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 9 Oct 2018 13:15:57 -0700
> 
> >> Any of these would insulate the rest of Emacs from this glitch.
> > The last two are undesirable, since it is generally expected of a
> > single Windows binary to run on all supported systems; having 2
> > separate binaries is possible, but complicates the matters.
> 
> 
> Alternative (2) should also let a single binary run on all supported 
> MS-Windows systems, unless I'm misunderstanding something.

You are right, I wasn't paying attention.

> The idea is that Emacs proper uses 64-bit time_t and only a small
> part of w32*.c knows whether the MS-Windows API is using 32- or
> 64-bit time_t. Emacs could do this by using "#define time_t long
> long int" for most of Emacs, and having only the small part of
> w32*.c worry about the conversion.

Yes.  Not sure about the "small" part, though: time_t appears in many
libc functions ('stat' and 'fstat' come to mind), and we currently
still use most of the structures defined in system headers which
reference time_t values.

> Also, don't we already have 2 separate binaries, one for 32-bit and one 
> for 64-bit MS-Windows?

We do, but the 32-bit binaries are expected to run on 64-bit systems.
We cannot avoid having the separate 64-bit binaries, whereas the
additional 32-bit binaries are just a nuisance.  Note that at least
some of the support libraries might also need to be built twice, if
time_t is used in their interfaces, directly or indirectly.

> > Last time this came up, we decided not to drop support even for Windows 9X
> 
> It's your decision since you're the maintainer, and if you want to spend 
> time porting to obsolete operating systems it's your time to spend. That 
> being said, there's vanishingly little real-world need to run the *very 
> latest* version of GNU Emacs on Windows XP and earlier and for security 
> reasons if the documentation for the latest Emacs version discusses 
> these older machines it should be warning Emacs users to not connect 
> these machines to the Internet.

Well, my main development machine still runs XP, so for now this is a
real necessity ;-)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.