ts.el icon indicating copy to clipboard operation
ts.el copied to clipboard

Merging ts.el into Emacs core

Open yantar92 opened this issue 1 year ago • 19 comments

Following up https://github.com/alphapapa/org-ql/issues/409

@alphapapa listed the following reasons why ts.el may be beneficial compared to the existing time API:

One purpose of the ts struct is to provide convenient accessors for elements of the timestamp without having to decode and format the time value repeatedly; whether that's an important enough benefit to keep may be debatable, but I did measure a significant performance benefit when I was writing it (and whether user code typically uses that pattern is, again, another matter).

The main purpose of ts is to provide a unified API. Emacs's default time/date functions are spread across various libraries and symbol prefixes, and various functions take different arguments and types, which becomes bewildering. The API's using a single data type is a significant benefit, as all of the functions can be composed, chained, etc.

  • [ ] For performance, it would be nice to provide actual benchmarks with numbers that we can present to Emacs devs.
  • [ ] For "unified API", it would be nice to provide examples demonstrating how the new API is better.

yantar92 avatar Feb 19 '24 11:02 yantar92

Various benchmarks from earlier in ts's development may be seen in the notes file: https://github.com/alphapapa/ts.el/blob/master/notes.org

alphapapa avatar Feb 19 '24 21:02 alphapapa

Adam Porter @.***> writes:

Various benchmarks from earlier in ts's development may be seen in the notes file: https://github.com/alphapapa/ts.el/blob/master/notes.org

I looked through your benchmarks and I cannot see how they justify a new time representation:

  1. https://github.com/alphapapa/ts.el/blob/master/notes.org#with-filling demonstrates list vs vector access. This will not be considered significant.
  2. https://github.com/alphapapa/ts.el/blob/master/notes.org#accessor-dispatch-vs-string-to-number-format-time-string is not fair. If you need to get year, (string-to-number (format-time-string "%Y" unix)) is very awkward way to do it. You should better use (decoded-time-year (decode-time (current-time)))

All other benchmarks does not seem relevant to comparison with the existing Emacs time APIs.

That said, new API functions introduced in ts.el might be of interest for upstream.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

yantar92 avatar Feb 20 '24 13:02 yantar92

Note, of course, that decoded-time- accessors did not exist in Emacs prior to ts. Those were added after ts had been released and used in the wild for some time.

That said, new API functions introduced in ts.el might be of interest for upstream.

At this point, I would expect that the appeal would be the unified API for accessing parts of and manipulating timestamps.

alphapapa avatar Feb 20 '24 23:02 alphapapa

Adam Porter @.***> writes:

Note, of course, that decoded-time- accessors did not exist in Emacs prior to ts. Those were added after ts had been released and used in the wild for some time.

Sure, but it does not matter if you want to merge ts.el upstream. We need to compare with Emacs master.

That said, new API functions introduced in ts.el might be of interest for upstream.

At this point, I would expect that the appeal would be the unified API for accessing parts of and manipulating timestamps.

I do not see how unified API would justify ts struct (there are already too many time representations in Emacs; adding yet another one will be questioned). It may be necessary to work with `decode-time'-returned struct instead.

Further, I suggest providing some kind of summary comparing the existing time API and what you change/add in ts.el. This will help to see which aspects need to be improved and show why a new set of functions is needed.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

yantar92 avatar Feb 21 '24 10:02 yantar92

I do not see how unified API would justify ts struct (there are already too many time representations in Emacs; adding yet another one will be questioned). It may be necessary to work with `decode-time'-returned struct instead.

I did not mean to propose upstreaming the struct, just the API.

Further, I suggest providing some kind of summary comparing the existing time API and what you change/add in ts.el. This will help to see which aspects need to be improved and show why a new set of functions is needed.

Of course, it would be unlikely to be accepted otherwise. However, this is one of the most time-consuming aspects of such a proposal, the "selling" and convincing via laborious examples, ones which seem obviously beneficial to those who need them, but will be repeatedly questioned by those who don't write related code and can't imagine the benefit.

Do you agree that having a unified API for working with time values, like the one in ts, is useful, and would be a good thing to have in Emacs? Or would I need to argue for this by myself?

As well, if a proposal were accepted upstream, would you be willing to help with writing the code and/or documentation?

Thanks.

alphapapa avatar Feb 21 '24 11:02 alphapapa

Adam Porter @.***> writes:

Do you agree that having a unified API for working with time values, like the one in ts, is useful, and would be a good thing to have in Emacs? Or would I need to argue for this by myself?

Org mode uses ad-hoc time API to work with time shifts and time in general. In particular, org-current-effective-time', org-fix-decoded-time', org-closest-date', org-table-time-seconds-to-string', org-time=/</>/<>/<=/>=', and org-read-date'.

This API is invented out of necessity. Org more would use native API if Emacs had one.

As well, if a proposal were accepted upstream, would you be willing to help with writing the code and/or documentation?

Sure, I can help. Especially to implement/document equivalents of functions and commands used in Org mode.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

yantar92 avatar Feb 21 '24 12:02 yantar92

Here's a very simple initial example of how the ts API is easier to use to manipulate a timestamp than the current Emacs native ones:

(let ((decoded (decode-time argh-time)))
  (cl-incf (decoded-time-day decoded) 365)
  (format-time-string "%c" (encode-time decoded)))
;; "Fri Feb 21 20:28:47 2025"

(ts-format "%c" (ts-adjust 'day 365 (ts-now)))
;; "Fri Feb 21 20:34:18 2025"

Also of note is the differing values: while I don't claim that ts's results are always necessarily "correct" for this kind of thing (as taking into account leap years, leap seconds, and etc. is non-trivial, and ts doesn't even try to do this itself), it's notable that the result of changing the decoded time day and then re-encoding it is off by about 6 minutes.

In fact, there seems to be some kind of weird overflow happening, because as I write this message and experiment with code that modifies the seconds slot instead of the day slot, I notice that the time in the result doesn't change, even though it's being evaluated a few minutes later:

(let ((decoded (decode-time argh-time)))
  (cl-incf (decoded-time-second decoded) (* 60 60 24 365))
  (format-time-string "%c" (encode-time decoded)))
;; "Fri Feb 21 20:28:47 2025"

So to get a more accurate result requires working on the time value in terms of seconds and internal time values, like:

(let ((time (current-time)))
  (setf time (time-add (seconds-to-time (* 60 60 24 365)) time))
  (format-time-string "%c" time))
;; "Fri Feb 21 20:39:52 2025"

Next to which ts seems rather appealing to me:

(ts-format "%c" (ts-adjust 'day 365 (ts-now)))
;; "Fri Feb 21 20:34:18 2025"

So the idea of refactoring ts to use time values instead of the ts struct, while preserving the API, seems somewhat appealing to me.

However, another matter to keep in mind is that, since ts uses Unix timestamps internally, it helps to deal with time-zone issues, as Unix timestamps are in UTC by definition; whereas Emacs's internal time values don't have a defined time zone, so users must either ensure they are converted to UTC or carry the associated time zone separately.

What do you think? Thanks.

alphapapa avatar Feb 23 '24 02:02 alphapapa

Adam Porter @.***> writes:

Here's a very simple initial example of how the ts API is easier to use to manipulate a timestamp than the current Emacs native ones:

(let ((decoded (decode-time argh-time)))
  (cl-incf (decoded-time-day decoded) 365)
  (format-time-string "%c" (encode-time decoded)))
;; "Fri Feb 21 20:28:47 2025"

(ts-format "%c" (ts-adjust 'day 365 (ts-now)))
;; "Fri Feb 21 20:34:18 2025"

I would not call it "unified API". `ts-adjust' is simply missing from Emacs built-in libraries. Proposing a set of new time-related functions to Emacs upstream is easier than proposing a new API, I think.

Also of note is the differing values: while I don't claim that ts's results are always necessarily "correct" for this kind of thing (as taking into account leap years, leap seconds, and etc. is non-trivial, and ts doesn't even try to do this itself),

I think it is trivial. But you need to use things like `time-add' that make use of C time libs that are aware about IANA timezone DB.

it's notable that the result of changing the decoded time day and then re-encoding it is off by about 6 minutes.

Which is likely because you assigned argh-time value 6 minutes before running the above code. If you instead use (current-time), you will get consistent results (as expected). At least, I do.

However, another matter to keep in mind is that, since ts uses Unix timestamps internally, it helps to deal with time-zone issues, as Unix timestamps are in UTC by definition; whereas Emacs's internal time values don't have a defined time zone, so users must either ensure they are converted to UTC or carry the associated time zone separately.

That's good point. Although the difference is not because ts' uses Unix timestamp, but because ts' explicitly stores time zone name. In contrast, `decode-time' only provides UTC offset and does not retain the time zone info. UTC offset alone is not enough to account for timezone-specific rules about month lengths and daylight saving time transitions.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

yantar92 avatar Feb 23 '24 12:02 yantar92

Ihor Radchenko @.***> writes:

However, another matter to keep in mind is that, since ts uses Unix timestamps internally, it helps to deal with time-zone issues, as Unix timestamps are in UTC by definition; whereas Emacs's internal time values don't have a defined time zone, so users must either ensure they are converted to UTC or carry the associated time zone separately.

That's good point. Although the difference is not because ts' uses Unix timestamp, but because ts' explicitly stores time zone name. In contrast, `decode-time' only provides UTC offset and does not retain the time zone info. UTC offset alone is not enough to account for timezone-specific rules about month lengths and daylight saving time transitions.

I was not accurate here. encode-time' actually understands non-offset time zones like "Asia/Singapore". It is just that decode-time' only returns the UTC offset. And tz-abbrv slot is not used in practice (so, it is hard to justify adding it).

To move things forward, let me provide a tentative list of functions that might be added to Emacs upstream:

ts-parse-fill, ts-apply, ts-human-duration, ts-human-format-duration, ts-adjust, ts-inc, ts-dec, ts-adjustf, ts-incf, ts-decf, ts-in, ts<=, ts>, ts>=.

-- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92

yantar92 avatar Mar 24 '24 14:03 yantar92

Let me know if I can help anything to move things forward.

yantar92 avatar Apr 24 '24 07:04 yantar92

I appreciate that. The next few weeks will be especially busy for me, and I won't have much time for Emacs-related things. Anyway, your tentative list seems good to me.

I suppose a next step might be to propose such an API to emacs-devel--that is, without the internal things that ts does regarding Unix timestamps and caching of formatted parts. If that could be agreed upon, then the implementation would be pretty simple and straightforward.

alphapapa avatar Apr 30 '24 18:04 alphapapa

It has been a few months. Just bumping the thread :)

yantar92 avatar Aug 07 '24 07:08 yantar92

Thanks for the ping, Ihor. Haven't had much time lately for Emacs things. Maybe I can find some soon.

alphapapa avatar Aug 08 '24 20:08 alphapapa