Question about bignum usage

Question about bignum usage

[Gerd Möllmann]

In the igc branch, I see that Emacs creates bigints in an rate that I
find astonishsing, thousands in a few minutes. Most backtraces look like
this.

  * frame #0: 0x00000001000f9ce8 emacs`make_integer_mpz [inlined] make_bignum_bits(bits=71) at bignum.c:99:7 [opt]
    frame #1: 0x00000001000f9ce8 emacs`make_integer_mpz at bignum.c:170:10 [opt]
    frame #2: 0x00000001001beb48 emacs`decode_time_components(form=TIMEFORM_HI_LO_US_PS, high=<unavailable>, low=<unavailable>, usec=<unavailable>, psec=<unavailable>, result=0x000000016fdfde50, dresult=0x0000000000000000) at timefns.c:812:27 [opt]
    frame #3: 0x00000001001be7e4 emacs`list4_to_timespec(high=<unavailable>, low=<unavailable>, usec=<unavailable>, psec=<unavailable>, result=0x000000016fdfdec0) at timefns.c:1008:7 [opt]
    frame #4: 0x00000001000e1c98 emacs`timer_check [inlined] decode_timer(timer=(struct Lisp_Vector *) $2 = 0x000000010ab42538, result=0x000000016fdfdec0) at keyboard.c:4648:10 [opt]
    frame #5: 0x00000001000e1c54 emacs`timer_check at keyboard.c:4713:10 [opt]
    frame #6: 0x00000001000e1b44 emacs`timer_check at keyboard.c:4856:18 [opt]
    frame #7: 0x00000001000df51c emacs`detect_input_pending_run_timers [inlined] readable_events(flags=1) at keyboard.c:3577:5 [opt]
    frame #8: 0x00000001000df518 emacs`detect_input_pending_run_timers [inlined] get_input_pending(flags=1) at keyboard.c:7859:42 [opt]
    frame #9: 0x00000001000df508 emacs`detect_input_pending_run_timers(do_display=true) at keyboard.c:11563:5 [opt]

Is this expected?

Re: Question about bignum usage

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Thu, 20 Jun 2024 07:59:36 +0200
> 
> In the igc branch, I see that Emacs creates bigints in an rate that I
> find astonishsing, thousands in a few minutes. Most backtraces look like
> this.
> 
>   * frame #0: 0x00000001000f9ce8 emacs`make_integer_mpz [inlined] make_bignum_bits(bits=71) at bignum.c:99:7 [opt]
>     frame #1: 0x00000001000f9ce8 emacs`make_integer_mpz at bignum.c:170:10 [opt]
>     frame #2: 0x00000001001beb48 emacs`decode_time_components(form=TIMEFORM_HI_LO_US_PS, high=<unavailable>, low=<unavailable>, usec=<unavailable>, psec=<unavailable>, result=0x000000016fdfde50, dresult=0x0000000000000000) at timefns.c:812:27 [opt]
>     frame #3: 0x00000001001be7e4 emacs`list4_to_timespec(high=<unavailable>, low=<unavailable>, usec=<unavailable>, psec=<unavailable>, result=0x000000016fdfdec0) at timefns.c:1008:7 [opt]
>     frame #4: 0x00000001000e1c98 emacs`timer_check [inlined] decode_timer(timer=(struct Lisp_Vector *) $2 = 0x000000010ab42538, result=0x000000016fdfdec0) at keyboard.c:4648:10 [opt]
>     frame #5: 0x00000001000e1c54 emacs`timer_check at keyboard.c:4713:10 [opt]
>     frame #6: 0x00000001000e1b44 emacs`timer_check at keyboard.c:4856:18 [opt]
>     frame #7: 0x00000001000df51c emacs`detect_input_pending_run_timers [inlined] readable_events(flags=1) at keyboard.c:3577:5 [opt]
>     frame #8: 0x00000001000df518 emacs`detect_input_pending_run_timers [inlined] get_input_pending(flags=1) at keyboard.c:7859:42 [opt]
>     frame #9: 0x00000001000df508 emacs`detect_input_pending_run_timers(do_display=true) at keyboard.c:11563:5 [opt]
> 
> Is this expected?

Why not?  Time values are known to produce bignums, yes.  What does
list-timers show on that system?

Re: Question about bignum usage

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Thu, 20 Jun 2024 07:59:36 +0200
>> 
>> In the igc branch, I see that Emacs creates bigints in an rate that I
>> find astonishsing, thousands in a few minutes. Most backtraces look like
>> this.
>> 
>>   * frame #0: 0x00000001000f9ce8 emacs`make_integer_mpz [inlined] make_bignum_bits(bits=71) at bignum.c:99:7 [opt]
>>     frame #1: 0x00000001000f9ce8 emacs`make_integer_mpz at bignum.c:170:10 [opt]
>>     frame #2: 0x00000001001beb48 emacs`decode_time_components(form=TIMEFORM_HI_LO_US_PS, high=<unavailable>, low=<unavailable>, usec=<unavailable>, psec=<unavailable>, result=0x000000016fdfde50, dresult=0x0000000000000000) at timefns.c:812:27 [opt]
>>     frame #3: 0x00000001001be7e4 emacs`list4_to_timespec(high=<unavailable>, low=<unavailable>, usec=<unavailable>, psec=<unavailable>, result=0x000000016fdfdec0) at timefns.c:1008:7 [opt]
>>     frame #4: 0x00000001000e1c98 emacs`timer_check [inlined] decode_timer(timer=(struct Lisp_Vector *) $2 = 0x000000010ab42538, result=0x000000016fdfdec0) at keyboard.c:4648:10 [opt]
>>     frame #5: 0x00000001000e1c54 emacs`timer_check at keyboard.c:4713:10 [opt]
>>     frame #6: 0x00000001000e1b44 emacs`timer_check at keyboard.c:4856:18 [opt]
>>     frame #7: 0x00000001000df51c emacs`detect_input_pending_run_timers [inlined] readable_events(flags=1) at keyboard.c:3577:5 [opt]
>>     frame #8: 0x00000001000df518 emacs`detect_input_pending_run_timers [inlined] get_input_pending(flags=1) at keyboard.c:7859:42 [opt]
>>     frame #9: 0x00000001000df508 emacs`detect_input_pending_run_timers(do_display=true) at keyboard.c:11563:5 [opt]
>> 
>> Is this expected?
>
> Why not?  Time values are known to produce bignums, yes.  

Ok.

> What does list-timers show on that system?

               1.0s         5.0s auto-revert-buffers
               5.4s            - undo-auto--boundary-timer
           4m 19.0s           5m persistent-scratch-save
   *           0.1s            t show-paren-function
   *           0.5s            t posframe-hidehandler-daemon-function
   *           0.5s            t #f(compiled-function () #<bytecode -0x181a979fe3c9ba30> [jit-lock--antiblink-grace-timer jit-lock-context-fontify])
   *           0.5s      :repeat blink-cursor-start
   *           1.0s            t which-key--update

Re: Question about bignum usage

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 20 Jun 2024 08:34:22 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Why not?  Time values are known to produce bignums, yes.  
> 
> Ok.
> 
> > What does list-timers show on that system?
> 
>                1.0s         5.0s auto-revert-buffers
>                5.4s            - undo-auto--boundary-timer
>            4m 19.0s           5m persistent-scratch-save
>    *           0.1s            t show-paren-function
>    *           0.5s            t posframe-hidehandler-daemon-function
>    *           0.5s            t #f(compiled-function () #<bytecode -0x181a979fe3c9ba30> [jit-lock--antiblink-grace-timer jit-lock-context-fontify])
>    *           0.5s      :repeat blink-cursor-start
>    *           1.0s            t which-key--update

This means we check for expired times every 100 msec, so yes, we will
create a lot of bignums.  I see something similar here as well.

Re: Question about bignum usage

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Thu, 20 Jun 2024 08:34:22 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Why not?  Time values are known to produce bignums, yes.  
>> 
>> Ok.
>> 
>> > What does list-timers show on that system?
>> 
>>                1.0s         5.0s auto-revert-buffers
>>                5.4s            - undo-auto--boundary-timer
>>            4m 19.0s           5m persistent-scratch-save
>>    *           0.1s            t show-paren-function
>>    *           0.5s            t posframe-hidehandler-daemon-function
>>    *           0.5s            t #f(compiled-function () #<bytecode -0x181a979fe3c9ba30> [jit-lock--antiblink-grace-timer jit-lock-context-fontify])
>>    *           0.5s      :repeat blink-cursor-start
>>    *           1.0s            t which-key--update
>
> This means we check for expired times every 100 msec, so yes, we will
> create a lot of bignums.  I see something similar here as well.

Thanks.

It isn't a problem, one just have to know why it happens.

In igc, these bignums accumulate pretty quickly to figures like

  PVEC_BIGNUM                              97199         3110368

and much higher, where the first number is the number of bignums, and
the second is bytes. The number of fonts is also curious.

  PVEC_FONT                                90422        10868392

But all seem to be GC'd eventually.

Re: Question about bignum usage

[Mattias Engdegård]

20 juni 2024 kl. 09.36 skrev Eli Zaretskii <eliz@gnu.org>:

> This means we check for expired times every 100 msec, so yes, we will
> create a lot of bignums.

Right, and a few hundred bignums allocated every second isn't something even the current GC should have any trouble with.

But it is a bit wasteful, isn't it? We use very short-lived bignums for internal purposes even when the picosecond part is 0, in places like decode_timer where there should be need to cons anything at all.

Re: Question about bignum usage

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Thu, 20 Jun 2024 11:38:48 +0200
> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>,
>  emacs-devel@gnu.org
> 
> 20 juni 2024 kl. 09.36 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > This means we check for expired times every 100 msec, so yes, we will
> > create a lot of bignums.
> 
> Right, and a few hundred bignums allocated every second isn't something even the current GC should have any trouble with.
> 
> But it is a bit wasteful, isn't it? We use very short-lived bignums for internal purposes even when the picosecond part is 0, in places like decode_timer where there should be need to cons anything at all.

Sure, if that can be avoided, it would be beneficial.  Paul, any
suggestions?

Re: Question about bignum usage

[Gerd Möllmann]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> the second is bytes. The number of fonts is also curious.
>
>   PVEC_FONT                                90422        10868392
>
> But all seem to be GC'd eventually.

FWIW, I'm attaching the Lisp that I currently use to display this stuff.
Something hacked together. M-x igc-stats displays a buffer in which one
can take two snapshots of igc-info, display them, sompare them, trigger
GC and so on.

The PVEC_FONT stuff is impressive :-) 


[-- Attachment #2: igc-stats.el --]
[-- Type: application/emacs-lisp, Size: 2983 bytes --]

Re: Question about bignum usage

[Helmut Eller]

On Thu, Jun 20 2024, Gerd Möllmann wrote:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> the second is bytes. The number of fonts is also curious.
>>
>>   PVEC_FONT                                90422        10868392
>>
>> But all seem to be GC'd eventually.
>
> FWIW, I'm attaching the Lisp that I currently use to display this stuff.
> Something hacked together. M-x igc-stats displays a buffer in which one
> can take two snapshots of igc-info, display them, sompare them, trigger
> GC and so on.

Thanks!  That's quite nice.

> The PVEC_FONT stuff is impressive :-) 

There don't seem to be so many PVEC_FONT objects here.  After
startup (with my normal .emacs):

PVEC_FONT                                 1251          151824

and then after opening igc.c and moving around a bit:

PVEC_FONT                                 1968          238312

Re: Question about bignum usage

[Gerd Möllmann]

Helmut Eller <eller.helmut@gmail.com> writes:

> On Thu, Jun 20 2024, Gerd Möllmann wrote:
>
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>
>>> the second is bytes. The number of fonts is also curious.
>>>
>>>   PVEC_FONT                                90422        10868392
>>>
>>> But all seem to be GC'd eventually.
>>
>> FWIW, I'm attaching the Lisp that I currently use to display this stuff.
>> Something hacked together. M-x igc-stats displays a buffer in which one
>> can take two snapshots of igc-info, display them, sompare them, trigger
>> GC and so on.
>
> Thanks!  That's quite nice.
>
>> The PVEC_FONT stuff is impressive :-) 
>
> There don't seem to be so many PVEC_FONT objects here.  After
> startup (with my normal .emacs):
>
> PVEC_FONT                                 1251          151824
>
> and then after opening igc.c and moving around a bit:
>
> PVEC_FONT                                 1968          238312

That's interesting! I'm also using my normal init.el, so I'm just using
the igc branch in the way I use Emacs day by day. It's apparently not a
problem, but I wonder what's going on... 

Re: Question about bignum usage

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Helmut Eller <eller.helmut@gmail.com> writes:
>
>> On Thu, Jun 20 2024, Gerd Möllmann wrote:
>>
>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>>
>>>> the second is bytes. The number of fonts is also curious.
>>>>
>>>>   PVEC_FONT                                90422        10868392
>>>>
>>>> But all seem to be GC'd eventually.
>>>
>>> FWIW, I'm attaching the Lisp that I currently use to display this stuff.
>>> Something hacked together. M-x igc-stats displays a buffer in which one
>>> can take two snapshots of igc-info, display them, sompare them, trigger
>>> GC and so on.
>>
>> Thanks!  That's quite nice.
>>
>>> The PVEC_FONT stuff is impressive :-) 
>>
>> There don't seem to be so many PVEC_FONT objects here.  After
>> startup (with my normal .emacs):
>>
>> PVEC_FONT                                 1251          151824
>>
>> and then after opening igc.c and moving around a bit:
>>
>> PVEC_FONT                                 1968          238312
>
> That's interesting! I'm also using my normal init.el, so I'm just using
> the igc branch in the way I use Emacs day by day. It's apparently not a
> problem, but I wonder what's going on... 

Seems to have something to do with a header line.

  * frame #0: 0x0000000100177d28 emacs`copy_font_spec [inlined] TAGGEDP(a=(struct font *) $1 = 0x000000010d749bb8, tag=Lisp_Vectorlike) at lisp.h:781:10 [opt]
    frame #1: 0x0000000100177d28 emacs`copy_font_spec [inlined] PSEUDOVECTORP(a=(struct font *) $2 = 0x000000010d749bb8, code=36) at lisp.h:1105:11 [opt]
    frame #2: 0x0000000100177d28 emacs`copy_font_spec [inlined] FONTP(x=(struct font *) $3 = 0x000000010d749bb8) at font.h:437:10 [opt]
    frame #3: 0x0000000100177d28 emacs`copy_font_spec [inlined] CHECK_FONT(x=(struct font *) $4 = 0x000000010d749bb8) at font.h:487:15 [opt]
    frame #4: 0x0000000100177d28 emacs`copy_font_spec(font=(struct font *) $5 = 0x000000010d749bb8) at font.c:4105:3 [opt]
!gud 4105:3:/Users/gerd/emacs/github/igc/src/font.c
    frame #5: 0x0000000100179898 emacs`font_clear_prop(attrs=(struct Lisp_Symbol *) $6 = 0x00000002706c9cf0, prop=FONT_WEIGHT_INDEX) at font.c:3088:12 [opt]
    frame #6: 0x00000001000c5c30 emacs`merge_face_vectors(w=<unavailable>, f=<unavailable>, from=(struct Lisp_Symbol *) $7 = 0x00000002706c97b8, to=(struct Lisp_Symbol *) $8 = 0x00000002706c9cf0, named_merge_points=<unavailable>) at xfaces.c:0 [opt]
    frame #7: 0x00000001000ccbcc emacs`merge_named_face(w=0x0000000106e41e28, f=0x0000000105c96c00, face_name=<unavailable>, to=(struct Lisp_Symbol *) $9 = 0x00000002706c9cf0, named_merge_points=<unavailable>, attr_filter=<no summary available>) at xfaces.c:2420:9 [opt]
    frame #8: 0x00000001000c93ec emacs`merge_face_ref(w=<unavailable>, f=0x0000000105c96c00, face_ref=(struct Lisp_Symbol *) $10 = 0x00000001008d7a60, to=(struct Lisp_Symbol *) $11 = 0x00000002706c9cf0, err_msgs=false, named_merge_points=0x000000016fdf6390, attr_filter=<no summary available>) at xfaces.c:2902:12 [opt]
    frame #9: 0x00000001000c5b70 emacs`merge_face_vectors(w=<unavailable>, f=<unavailable>, from=(struct Lisp_Symbol *) $12 = 0x00000002706c9a88, to=(struct Lisp_Symbol *) $13 = 0x00000002706c9cf0, named_merge_points=<unavailable>) at xfaces.c:2252:5 [opt]
    frame #10: 0x00000001000ccbcc emacs`merge_named_face(w=0x0000000106e41e28, f=0x0000000105c96c00, face_name=<unavailable>, to=(struct Lisp_Symbol *) $14 = 0x00000002706c9cf0, named_merge_points=<unavailable>, attr_filter=<no summary available>) at xfaces.c:2420:9 [opt]
    frame #11: 0x00000001000c93ec emacs`merge_face_ref(w=<unavailable>, f=0x0000000105c96c00, face_ref=(struct Lisp_Symbol *) $15 = 0x000000010adfa890, to=(struct Lisp_Symbol *) $16 = 0x00000002706c9cf0, err_msgs=true, named_merge_points=0x0000000000000000, attr_filter=<no summary available>) at xfaces.c:2902:12 [opt]
    frame #12: 0x00000001000cc564 emacs`face_at_string_position(w=0x0000000106e41e28, string=(struct Lisp_String *) $17 = 0x000000010adfd4a0, pos=1, bufpos=0, endptr=<unavailable>, base_face_id=<unavailable>, mouse_p=<unavailable>, attr_filter=<no summary available>) at xfaces.c:7044:5 [opt]
    frame #13: 0x00000001000376d4 emacs`display_string(string="*igc*", lisp_string=(struct Lisp_String *) $18 = 0x0000000106e3cf20, face_string=(struct Lisp_String *) $19 = 0x000000010adfd4a0, face_string_pos=1, start=<unavailable>, it=0x000000016fdf7e70, field_width=<unavailable>, precision=<unavailable>, max_x=0, multibyte=0) at xdisp.c:29122:4 [opt]
    frame #14: 0x0000000100030a38 emacs`display_mode_element(it=0x000000016fdf7e70, depth=<unavailable>, field_width=0, precision=-10, elt=(struct Lisp_String *) $20 = 0x000000010adfd4a0, props=(struct Lisp_Symbol *) $21 = 0x00000001008d36e0, risky=<unavailable>) at xdisp.c:27793:17 [opt]
    frame #15: 0x000000010002f4b8 emacs`display_mode_element(it=0x000000016fdf7e70, depth=<unavailable>, field_width=0, precision=-10, elt=<unavailable>, props=(struct Lisp_Symbol *) $22 = 0x00000001008d36e0, risky=<unavailable>) at xdisp.c:27970:13 [opt]
    frame #16: 0x000000010002f4b8 emacs`display_mode_element(it=0x000000016fdf7e70, depth=<unavailable>, field_width=0, precision=0, elt=<unavailable>, props=(struct Lisp_Symbol *) $23 = 0x00000001008d36e0, risky=<unavailable>) at xdisp.c:27970:13 [opt]
    frame #17: 0x000000010001dad4 emacs`display_mode_line(w=0x0000000106e41e28, face_id=MODE_LINE_ACTIVE_FACE_ID, format=(struct Lisp_Cons *) $24 = 0x000000010a267750) at xdisp.c:27395:7 [opt]
    frame #18: 0x0000000100052740 emacs`display_mode_lines(w=0x0000000106e41e28) at xdisp.c:27308:7 [opt]
    frame #19: 0x000000010005a5ac emacs`redisplay_window(window=(struct window *) $25 = 0x0000000106e41e28, just_this_one_p=false) at xdisp.c:20927:7 [opt]

It's always copy_font_spec that is involved in this way. I can lean on
'g' in the *igc* buffer, and watch is grow by the hundreds :-)

Re: Question about bignum usage

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Helmut Eller <eller.helmut@gmail.com> writes:
>>
>>> On Thu, Jun 20 2024, Gerd Möllmann wrote:
>>>
>>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>>>
>>>>> the second is bytes. The number of fonts is also curious.
>>>>>
>>>>>   PVEC_FONT                                90422        10868392
>>>>>
>>>>> But all seem to be GC'd eventually.
>>>>
>>>> FWIW, I'm attaching the Lisp that I currently use to display this stuff.
>>>> Something hacked together. M-x igc-stats displays a buffer in which one
>>>> can take two snapshots of igc-info, display them, sompare them, trigger
>>>> GC and so on.
>>>
>>> Thanks!  That's quite nice.
>>>
>>>> The PVEC_FONT stuff is impressive :-) 
>>>
>>> There don't seem to be so many PVEC_FONT objects here.  After
>>> startup (with my normal .emacs):
>>>
>>> PVEC_FONT                                 1251          151824
>>>
>>> and then after opening igc.c and moving around a bit:
>>>
>>> PVEC_FONT                                 1968          238312
>>
>> That's interesting! I'm also using my normal init.el, so I'm just using
>> the igc branch in the way I use Emacs day by day. It's apparently not a
>> problem, but I wonder what's going on... 
>
> Seems to have something to do with a header line.
>
>   * frame #0: 0x0000000100177d28 emacs`copy_font_spec [inlined] TAGGEDP(a=(struct font *) $1 = 0x000000010d749bb8, tag=Lisp_Vectorlike) at lisp.h:781:10 [opt]
>     frame #1: 0x0000000100177d28 emacs`copy_font_spec [inlined] PSEUDOVECTORP(a=(struct font *) $2 = 0x000000010d749bb8, code=36) at lisp.h:1105:11 [opt]
>     frame #2: 0x0000000100177d28 emacs`copy_font_spec [inlined] FONTP(x=(struct font *) $3 = 0x000000010d749bb8) at font.h:437:10 [opt]
>     frame #3: 0x0000000100177d28 emacs`copy_font_spec [inlined] CHECK_FONT(x=(struct font *) $4 = 0x000000010d749bb8) at font.h:487:15 [opt]
>     frame #4: 0x0000000100177d28 emacs`copy_font_spec(font=(struct font *) $5 = 0x000000010d749bb8) at font.c:4105:3 [opt]
> !gud 4105:3:/Users/gerd/emacs/github/igc/src/font.c
>     frame #5: 0x0000000100179898 emacs`font_clear_prop(attrs=(struct Lisp_Symbol *) $6 = 0x00000002706c9cf0, prop=FONT_WEIGHT_INDEX) at font.c:3088:12 [opt]
>     frame #6: 0x00000001000c5c30 emacs`merge_face_vectors(w=<unavailable>, f=<unavailable>, from=(struct Lisp_Symbol *) $7 = 0x00000002706c97b8, to=(struct Lisp_Symbol *) $8 = 0x00000002706c9cf0, named_merge_points=<unavailable>) at xfaces.c:0 [opt]
>     frame #7: 0x00000001000ccbcc emacs`merge_named_face(w=0x0000000106e41e28, f=0x0000000105c96c00, face_name=<unavailable>, to=(struct Lisp_Symbol *) $9 = 0x00000002706c9cf0, named_merge_points=<unavailable>, attr_filter=<no summary available>) at xfaces.c:2420:9 [opt]
>     frame #8: 0x00000001000c93ec emacs`merge_face_ref(w=<unavailable>, f=0x0000000105c96c00, face_ref=(struct Lisp_Symbol *) $10 = 0x00000001008d7a60, to=(struct Lisp_Symbol *) $11 = 0x00000002706c9cf0, err_msgs=false, named_merge_points=0x000000016fdf6390, attr_filter=<no summary available>) at xfaces.c:2902:12 [opt]
>     frame #9: 0x00000001000c5b70 emacs`merge_face_vectors(w=<unavailable>, f=<unavailable>, from=(struct Lisp_Symbol *) $12 = 0x00000002706c9a88, to=(struct Lisp_Symbol *) $13 = 0x00000002706c9cf0, named_merge_points=<unavailable>) at xfaces.c:2252:5 [opt]
>     frame #10: 0x00000001000ccbcc emacs`merge_named_face(w=0x0000000106e41e28, f=0x0000000105c96c00, face_name=<unavailable>, to=(struct Lisp_Symbol *) $14 = 0x00000002706c9cf0, named_merge_points=<unavailable>, attr_filter=<no summary available>) at xfaces.c:2420:9 [opt]
>     frame #11: 0x00000001000c93ec emacs`merge_face_ref(w=<unavailable>, f=0x0000000105c96c00, face_ref=(struct Lisp_Symbol *) $15 = 0x000000010adfa890, to=(struct Lisp_Symbol *) $16 = 0x00000002706c9cf0, err_msgs=true, named_merge_points=0x0000000000000000, attr_filter=<no summary available>) at xfaces.c:2902:12 [opt]
>     frame #12: 0x00000001000cc564 emacs`face_at_string_position(w=0x0000000106e41e28, string=(struct Lisp_String *) $17 = 0x000000010adfd4a0, pos=1, bufpos=0, endptr=<unavailable>, base_face_id=<unavailable>, mouse_p=<unavailable>, attr_filter=<no summary available>) at xfaces.c:7044:5 [opt]
>     frame #13: 0x00000001000376d4 emacs`display_string(string="*igc*", lisp_string=(struct Lisp_String *) $18 = 0x0000000106e3cf20, face_string=(struct Lisp_String *) $19 = 0x000000010adfd4a0, face_string_pos=1, start=<unavailable>, it=0x000000016fdf7e70, field_width=<unavailable>, precision=<unavailable>, max_x=0, multibyte=0) at xdisp.c:29122:4 [opt]
>     frame #14: 0x0000000100030a38 emacs`display_mode_element(it=0x000000016fdf7e70, depth=<unavailable>, field_width=0, precision=-10, elt=(struct Lisp_String *) $20 = 0x000000010adfd4a0, props=(struct Lisp_Symbol *) $21 = 0x00000001008d36e0, risky=<unavailable>) at xdisp.c:27793:17 [opt]
>     frame #15: 0x000000010002f4b8 emacs`display_mode_element(it=0x000000016fdf7e70, depth=<unavailable>, field_width=0, precision=-10, elt=<unavailable>, props=(struct Lisp_Symbol *) $22 = 0x00000001008d36e0, risky=<unavailable>) at xdisp.c:27970:13 [opt]
>     frame #16: 0x000000010002f4b8 emacs`display_mode_element(it=0x000000016fdf7e70, depth=<unavailable>, field_width=0, precision=0, elt=<unavailable>, props=(struct Lisp_Symbol *) $23 = 0x00000001008d36e0, risky=<unavailable>) at xdisp.c:27970:13 [opt]
>     frame #17: 0x000000010001dad4 emacs`display_mode_line(w=0x0000000106e41e28, face_id=MODE_LINE_ACTIVE_FACE_ID, format=(struct Lisp_Cons *) $24 = 0x000000010a267750) at xdisp.c:27395:7 [opt]
>     frame #18: 0x0000000100052740 emacs`display_mode_lines(w=0x0000000106e41e28) at xdisp.c:27308:7 [opt]
>     frame #19: 0x000000010005a5ac emacs`redisplay_window(window=(struct window *) $25 = 0x0000000106e41e28, just_this_one_p=false) at xdisp.c:20927:7 [opt]
>
> It's always copy_font_spec that is involved in this way. I can lean on
> 'g' in the *igc* buffer, and watch is grow by the hundreds :-)

Maybe it's my use of modus-themes. But whatever, enough played :-)

  Face: header-line (sample) (customize this face)

  Documentation:
  Basic header-line face.

  Defined in ‘faces.el’.


             Family: unspecified
            Foundry: unspecified
              Width: unspecified
             Height: unspecified
             Weight: unspecified
              Slant: unspecified
         Foreground: unspecified
  DistantForeground: unspecified
         Background: #1e1e1e
          Underline: unspecified
           Overline: unspecified
     Strike-through: unspecified
                Box: unspecified
            Inverse: unspecified
            Stipple: unspecified
               Font: unspecified
            Fontset: unspecified
             Extend: unspecified
            Inherit: modus-themes-ui-variable-pitch

    This face was introduced, or its default value was changed, in
    version 21.1 of Emacs.

Re: Question about bignum usage

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Thu, 20 Jun 2024 20:32:00 +0200
> 
> Seems to have something to do with a header line.

You mean, mode line, right?  Because:

>     frame #17: 0x000000010001dad4 emacs`display_mode_line(w=0x0000000106e41e28, face_id=MODE_LINE_ACTIVE_FACE_ID, format=(struct Lisp_Cons *) $24 = 0x000000010a267750) at xdisp.c:27395:7 [opt]

MODE_LINE_ACTIVE_FACE_ID is for the mode line, not for the header
line.

> It's always copy_font_spec that is involved in this way. I can lean on
> 'g' in the *igc* buffer, and watch is grow by the hundreds :-)

Does the "*igc*" string have face or mouse-face text properties?  If
not, I don't understand why face_at_string_position calls
merge_face_ref.

Re: Question about bignum usage

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
>> Date: Thu, 20 Jun 2024 20:32:00 +0200
>> 
>> Seems to have something to do with a header line.
>
> You mean, mode line, right?  Because:
>
>>     frame #17: 0x000000010001dad4 emacs`display_mode_line(w=0x0000000106e41e28, face_id=MODE_LINE_ACTIVE_FACE_ID, format=(struct Lisp_Cons *) $24 = 0x000000010a267750) at xdisp.c:27395:7 [opt]
>
> MODE_LINE_ACTIVE_FACE_ID is for the mode line, not for the header
> line.
>
>> It's always copy_font_spec that is involved in this way. I can lean on
>> 'g' in the *igc* buffer, and watch is grow by the hundreds :-)
>
> Does the "*igc*" string have face or mouse-face text properties?  If
> not, I don't understand why face_at_string_position calls
> merge_face_ref.

Oh right, I've overlooked the id, sorry.

The "*igc*" is the buffer name. It doesn't have properties that I put on
it. And the mode-line face looks almost like the header-line face,
except the colors.

  Face: mode-line (sample) (customize this face)

  Documentation:
  Face for the mode lines as well as header lines.
  See ‘mode-line-active’ and ‘mode-line-inactive’ for the faces
  used on mode lines.

  Defined in ‘faces.el’.


             Family: unspecified
            Foundry: unspecified
              Width: unspecified
             Height: unspecified
             Weight: unspecified
              Slant: unspecified
         Foreground: #ffffff
  DistantForeground: unspecified
         Background: #505050
          Underline: unspecified
           Overline: unspecified
     Strike-through: unspecified
                Box: #959595
            Inverse: unspecified
            Stipple: unspecified
               Font: unspecified
            Fontset: unspecified
             Extend: unspecified
            Inherit: modus-themes-ui-variable-pitch

    This face was introduced, or its default value was changed, in
    version 21.1 of Emacs.

  [back]

Re: Question about bignum usage

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Thu, 20 Jun 2024 21:16:40 +0200
> 
> The "*igc*" is the buffer name. It doesn't have properties that I put on
> it.

If it's the buffer name, then mode-line-format puts the mouse-face on
it, to make it mouse-sensitive.  Which probably explains why we call
merge_face_ref.

Re: Question about bignum usage

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: eller.helmut@gmail.com,  emacs-devel@gnu.org
>> Date: Thu, 20 Jun 2024 21:16:40 +0200
>> 
>> The "*igc*" is the buffer name. It doesn't have properties that I put on
>> it.
>
> If it's the buffer name, then mode-line-format puts the mouse-face on
> it, to make it mouse-sensitive.  Which probably explains why we call
> merge_face_ref.

That makes sense, thanks!

Re: Question about bignum usage

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

On 6/20/24 12:02, Eli Zaretskii wrote:
>> From: Mattias Engdegård <mattias.engdegard@gmail.com>
>> Date: Thu, 20 Jun 2024 11:38:48 +0200
>>
>> 20 juni 2024 kl. 09.36 skrev Eli Zaretskii <eliz@gnu.org>:
>>
>>> This means we check for expired times every 100 msec, so yes, we will
>>> create a lot of bignums.
>>
>> Right, and a few hundred bignums allocated every second isn't something even the current GC should have any trouble with.
>>
>> But it is a bit wasteful, isn't it? We use very short-lived bignums for internal purposes even when the picosecond part is 0, in places like decode_timer where there should be need to cons anything at all.
> 
> Sure, if that can be avoided, it would be beneficial.  Paul, any
> suggestions?

I installed the attached. 0017 should should address the performance 
problem with decode_timer. The other patches refactor and address some 
related issues.

As I understand it decode_timer was the locus of the performance issue. 
If other places are also involved, please let me know.


[-- Attachment #2: 0001-Refactor-timefns-more-functionally.patch --]
[-- Type: text/x-patch, Size: 21236 bytes --]

From a7fee79aeba5e0d5bf8d17b92b93be4a94516219 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 6 Jul 2024 21:52:08 +0200
Subject: [PATCH 01/17] Refactor timefns more functionally

Use a more-functional style in timefns.c, rather than passing
pointers to objects that are filled in.  Although this does not
change behavior, it should help future improvements to the code.
* src/keyboard.c (decode_timer): Return a possibly-invalid struct
timespec instead of storing a timespec into a location specified
by an arg, and returning bool.  All callers changed.
* src/systime.h (struct lisp_time): Move from here to src/timefns.c,
since the type is private to timefns.c.
* src/timefns.c (decode_float_time, decode_ticks_hz):
Return timestamp instead of storing it into a location specified
by an arg.  All callers changed.
(enum cform, union c_time, struct err_time, struct form_time):
New types, to aid functional style.
(decode_time_components): Return struct err_time
instead of returning err and storing timestamp into a location
specified by an arg.  New arg cform.  All callers changed.
(decode_lisp_time): Return struct form_time instead of returning
form and storing timestamp into a location specified by an arg.
New arg cform, replacing decode_secs_only.  All callers changed.
(list4_to_timespec): Return possibly-invalid timestamp instead
of returning a bool and storing timestamp into a location specified
by an arg.  All callers changed.
(lisp_time_struct): Omit no-longer-needed arg PFORM.
All callers changed.
---
 src/keyboard.c |  22 ++--
 src/systime.h  |  14 +--
 src/timefns.c  | 317 ++++++++++++++++++++++++++-----------------------
 3 files changed, 184 insertions(+), 169 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index c75e80d2a05..40276b4157c 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -4646,20 +4646,21 @@ timer_resume_idle (void)
    ...).  Each element has the form (FUN . ARGS).  */
 Lisp_Object pending_funcalls;
 
-/* Return true if TIMER is a valid timer, placing its value into *RESULT.  */
-static bool
-decode_timer (Lisp_Object timer, struct timespec *result)
+/* Return the value of TIMER if it is a valid timer, an invalid struct
+   timespec otherwise.  */
+static struct timespec
+decode_timer (Lisp_Object timer)
 {
   Lisp_Object *vec;
 
   if (! (VECTORP (timer) && ASIZE (timer) == 10))
-    return false;
+    return invalid_timespec ();
   vec = XVECTOR (timer)->contents;
   if (! NILP (vec[0]))
-    return false;
+    return invalid_timespec ();
   if (! FIXNUMP (vec[2]))
-    return false;
-  return list4_to_timespec (vec[1], vec[2], vec[3], vec[8], result);
+    return invalid_timespec ();
+  return list4_to_timespec (vec[1], vec[2], vec[3], vec[8]);
 }
 
 
@@ -4706,7 +4707,6 @@ timer_check_2 (Lisp_Object timers, Lisp_Object idle_timers)
   while (CONSP (timers) || CONSP (idle_timers))
     {
       Lisp_Object timer = Qnil, idle_timer = Qnil;
-      struct timespec timer_time, idle_timer_time;
       struct timespec difference;
       struct timespec timer_difference = invalid_timespec ();
       struct timespec idle_timer_difference = invalid_timespec ();
@@ -4720,7 +4720,8 @@ timer_check_2 (Lisp_Object timers, Lisp_Object idle_timers)
       if (CONSP (timers))
 	{
 	  timer = XCAR (timers);
-	  if (! decode_timer (timer, &timer_time))
+	  struct timespec timer_time = decode_timer (timer);
+	  if (! timespec_valid_p (timer_time))
 	    {
 	      timers = XCDR (timers);
 	      continue;
@@ -4737,7 +4738,8 @@ timer_check_2 (Lisp_Object timers, Lisp_Object idle_timers)
       if (CONSP (idle_timers))
 	{
 	  idle_timer = XCAR (idle_timers);
-	  if (! decode_timer (idle_timer, &idle_timer_time))
+	  struct timespec idle_timer_time = decode_timer (idle_timer);
+	  if (! timespec_valid_p (idle_timer_time))
 	    {
 	      idle_timers = XCDR (idle_timers);
 	      continue;
diff --git a/src/systime.h b/src/systime.h
index fc93ea03233..1353c7158d0 100644
--- a/src/systime.h
+++ b/src/systime.h
@@ -77,22 +77,12 @@ timespec_valid_p (struct timespec t)
    (HI << LO_TIME_BITS) + LO + US / 1e6 + PS / 1e12.  */
 enum { LO_TIME_BITS = 16 };
 
-/* Components of a new-format Lisp timestamp.  */
-struct lisp_time
-{
-  /* Clock count as a Lisp integer.  */
-  Lisp_Object ticks;
-
-  /* Clock frequency (ticks per second) as a positive Lisp integer.  */
-  Lisp_Object hz;
-};
-
 /* defined in timefns.c */
 extern struct timeval make_timeval (struct timespec) ATTRIBUTE_CONST;
 extern Lisp_Object make_lisp_time (struct timespec);
 extern Lisp_Object timespec_to_lisp (struct timespec);
-extern bool list4_to_timespec (Lisp_Object, Lisp_Object, Lisp_Object,
-			       Lisp_Object, struct timespec *);
+extern struct timespec list4_to_timespec (Lisp_Object, Lisp_Object,
+					  Lisp_Object, Lisp_Object);
 extern struct timespec lisp_time_argument (Lisp_Object);
 extern double float_time (Lisp_Object);
 extern void init_timefns (void);
diff --git a/src/timefns.c b/src/timefns.c
index 746e422ffb6..70961c1a560 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -400,10 +400,21 @@ lo_time (time_t t)
    equals FLT_RADIX**P.  */
 static Lisp_Object flt_radix_power;
 
-/* Convert the finite number T into an Emacs time *RESULT, truncating
+/* Components of a Lisp timestamp (TICKS . HZ).  Using this C struct can
+   avoid the consing overhead of creating (TICKS . HZ).  */
+struct lisp_time
+{
+  /* Clock count as a Lisp integer.  */
+  Lisp_Object ticks;
+
+  /* Clock frequency (ticks per second) as a positive Lisp integer.  */
+  Lisp_Object hz;
+};
+
+/* Convert the finite number T into an Emacs time, truncating
    toward minus infinity.  Signal an error if unsuccessful.  */
-static void
-decode_float_time (double t, struct lisp_time *result)
+static struct lisp_time
+decode_float_time (double t)
 {
   Lisp_Object ticks, hz;
   if (t == 0)
@@ -447,8 +458,7 @@ decode_float_time (double t, struct lisp_time *result)
 	  ASET (flt_radix_power, scale, hz);
 	}
     }
-  result->ticks = ticks;
-  result->hz = hz;
+  return (struct lisp_time) { .ticks = ticks, .hz = hz };
 }
 
 /* Make a 4-element timestamp (HI LO US PS) from TICKS and HZ.
@@ -688,28 +698,39 @@ frac_to_double (Lisp_Object numerator, Lisp_Object denominator)
   return scalbn (mpz_get_d (*q), -scale);
 }
 
-/* From a valid timestamp (TICKS . HZ), generate the corresponding
-   time values.
+/* C timestamp forms.  This enum is passed to conversion functions to
+   specify the desired C timestamp form.  */
+enum cform
+  {
+    CFORM_TICKS_HZ, /* struct lisp_time */
+    CFORM_SECS_ONLY, /* struct lisp_time but HZ is 1 */
+    CFORM_DOUBLE /* double */
+  };
 
-   If RESULT is not null, store into *RESULT the converted time.
-   Otherwise, store into *DRESULT the number of seconds since the
-   start of the POSIX Epoch.
+/* A C timestamp in one of the forms specified by enum cform.  */
+union c_time
+{
+  struct lisp_time lt;
+  double d;
+};
 
-   Return zero, which indicates success.  */
-static int
-decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz,
-		 struct lisp_time *result, double *dresult)
+/* From a valid timestamp (TICKS . HZ), generate the corresponding
+   time value in CFORM form.  */
+static union c_time
+decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz, enum cform cform)
 {
-  if (result)
-    {
-      result->ticks = ticks;
-      result->hz = hz;
-    }
-  else
-    *dresult = frac_to_double (ticks, hz);
-  return 0;
+  return (cform == CFORM_DOUBLE
+	  ? (union c_time) { .d = frac_to_double (ticks, hz) }
+	  : (union c_time) { .lt = { .ticks = ticks, .hz = hz } });
 }
 
+/* An (error number, C timestamp) pair.  */
+struct err_time
+{
+  int err;
+  union c_time time;
+};
+
 /* Lisp timestamp classification.  */
 enum timeform
   {
@@ -723,111 +744,117 @@ decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz,
   };
 
 /* From the non-float form FORM and the time components HIGH, LOW, USEC
-   and PSEC, generate the corresponding time value.  If LOW is
+   and PSEC, generate the corresponding time value in CFORM form.  If LOW is
    floating point, the other components should be zero and FORM should
    not be TIMEFORM_TICKS_HZ.
 
-   If RESULT is not null, store into *RESULT the converted time.
-   Otherwise, store into *DRESULT the number of seconds since the
-   start of the POSIX Epoch.  Unsuccessful calls may or may not store
-   results.
-
-   Return zero if successful, an error number otherwise.  */
-static int
+   Return a (0, valid timestamp) pair if successful, an (error number,
+   unspecified timestamp) pair otherwise.  */
+static struct err_time
 decode_time_components (enum timeform form,
 			Lisp_Object high, Lisp_Object low,
 			Lisp_Object usec, Lisp_Object psec,
-			struct lisp_time *result, double *dresult)
+			enum cform cform)
 {
+  Lisp_Object ticks, hz;
+
   switch (form)
     {
     case TIMEFORM_INVALID:
-      return EINVAL;
+      return (struct err_time) { .err = EINVAL };
 
     case TIMEFORM_TICKS_HZ:
-      if (INTEGERP (high)
-	  && !NILP (Fnatnump (low)) && !BASE_EQ (low, make_fixnum (0)))
-	return decode_ticks_hz (high, low, result, dresult);
-      return EINVAL;
+      if (! (INTEGERP (high)
+	     && !NILP (Fnatnump (low)) && !BASE_EQ (low, make_fixnum (0))))
+	return (struct err_time) { .err = EINVAL };
+      ticks = high;
+      hz = low;
+      break;
 
     case TIMEFORM_FLOAT:
       eassume (false);
 
     case TIMEFORM_NIL:
-      return decode_ticks_hz (timespec_ticks (current_timespec ()),
-			      timespec_hz, result, dresult);
-
-    default:
+      ticks = timespec_ticks (current_timespec ());
+      hz = timespec_hz;
       break;
-    }
-
-  if (! (INTEGERP (high) && INTEGERP (low)
-	 && FIXNUMP (usec) && FIXNUMP (psec)))
-    return EINVAL;
-  EMACS_INT us = XFIXNUM (usec);
-  EMACS_INT ps = XFIXNUM (psec);
-
-  /* Normalize out-of-range lower-order components by carrying
-     each overflow into the next higher-order component.  */
-  us += ps / 1000000 - (ps % 1000000 < 0);
-  mpz_t *s = &mpz[1];
-  mpz_set_intmax (*s, us / 1000000 - (us % 1000000 < 0));
-  mpz_add (*s, *s, *bignum_integer (&mpz[0], low));
-  mpz_addmul_ui (*s, *bignum_integer (&mpz[0], high), 1 << LO_TIME_BITS);
-  ps = ps % 1000000 + 1000000 * (ps % 1000000 < 0);
-  us = us % 1000000 + 1000000 * (us % 1000000 < 0);
 
-  Lisp_Object hz;
-  switch (form)
-    {
-    case TIMEFORM_HI_LO:
-      /* Floats and nil were handled above, so it was an integer.  */
-      mpz_swap (mpz[0], *s);
-      hz = make_fixnum (1);
-      break;
-
-    case TIMEFORM_HI_LO_US:
-      mpz_set_ui (mpz[0], us);
-      mpz_addmul_ui (mpz[0], *s, 1000000);
-      hz = make_fixnum (1000000);
-      break;
+    default:
+      if (! (INTEGERP (high) && INTEGERP (low)
+	     && FIXNUMP (usec) && FIXNUMP (psec)))
+	return (struct err_time) { .err = EINVAL };
 
-    case TIMEFORM_HI_LO_US_PS:
       {
-	#if FASTER_TIMEFNS && TRILLION <= ULONG_MAX
-	  unsigned long i = us;
-	  mpz_set_ui (mpz[0], i * 1000000 + ps);
-	  mpz_addmul_ui (mpz[0], *s, TRILLION);
-	#else
-	  intmax_t i = us;
-	  mpz_set_intmax (mpz[0], i * 1000000 + ps);
-	  mpz_addmul (mpz[0], *s, ztrillion);
-	#endif
-	hz = trillion;
+	EMACS_INT us = XFIXNUM (usec);
+	EMACS_INT ps = XFIXNUM (psec);
+
+	/* Normalize out-of-range lower-order components by carrying
+	   each overflow into the next higher-order component.  */
+	us += ps / 1000000 - (ps % 1000000 < 0);
+	mpz_t *s = &mpz[1];
+	mpz_set_intmax (*s, us / 1000000 - (us % 1000000 < 0));
+	mpz_add (*s, *s, *bignum_integer (&mpz[0], low));
+	mpz_addmul_ui (*s, *bignum_integer (&mpz[0], high), 1 << LO_TIME_BITS);
+	ps = ps % 1000000 + 1000000 * (ps % 1000000 < 0);
+	us = us % 1000000 + 1000000 * (us % 1000000 < 0);
+
+	switch (form)
+	  {
+	  case TIMEFORM_HI_LO:
+	    /* Floats and nil were handled above, so it was an integer.  */
+	    mpz_swap (mpz[0], *s);
+	    hz = make_fixnum (1);
+	    break;
+
+	  case TIMEFORM_HI_LO_US:
+	    mpz_set_ui (mpz[0], us);
+	    mpz_addmul_ui (mpz[0], *s, 1000000);
+	    hz = make_fixnum (1000000);
+	    break;
+
+	  case TIMEFORM_HI_LO_US_PS:
+	    {
+	      #if FASTER_TIMEFNS && TRILLION <= ULONG_MAX
+		unsigned long i = us;
+		mpz_set_ui (mpz[0], i * 1000000 + ps);
+		mpz_addmul_ui (mpz[0], *s, TRILLION);
+	      #else
+		intmax_t i = us;
+		mpz_set_intmax (mpz[0], i * 1000000 + ps);
+		mpz_addmul (mpz[0], *s, ztrillion);
+	      #endif
+	      hz = trillion;
+	    }
+	    break;
+
+	  default:
+	    eassume (false);
+	  }
+	ticks = make_integer_mpz ();
       }
       break;
-
-    default:
-      eassume (false);
     }
 
-  return decode_ticks_hz (make_integer_mpz (), hz, result, dresult);
+  return (struct err_time) { .time = decode_ticks_hz (ticks, hz, cform) };
 }
 
+/* A (Lisp timeform, C timestamp) pair.  */
+struct form_time
+{
+  enum timeform form;
+  union c_time time;
+};
+
 /* Decode a Lisp timestamp SPECIFIED_TIME that represents a time.
 
-   If DECODE_SECS_ONLY, ignore and do not validate any sub-second
+   Return a (form, time) pair that is the form of SPECIFIED-TIME
+   and the resulting C timestamp in CFORM form.
+   If CFORM == CFORM_SECS_ONLY, ignore and do not validate any sub-second
    components of an old-format SPECIFIED_TIME.
 
-   If RESULT is not null, store into *RESULT the converted time;
-   otherwise, store into *DRESULT the number of seconds since the
-   start of the POSIX Epoch.  Unsuccessful calls may or may not store
-   results.
-
-   Return the form of SPECIFIED-TIME.  Signal an error if unsuccessful.  */
-static enum timeform
-decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
-		  struct lisp_time *result, double *dresult)
+   Signal an error if unsuccessful.  */
+static struct form_time
+decode_lisp_time (Lisp_Object specified_time, enum cform cform)
 {
   Lisp_Object high = make_fixnum (0);
   Lisp_Object low = specified_time;
@@ -845,7 +872,7 @@ decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
 	{
 	  Lisp_Object low_tail = XCDR (low);
 	  low = XCAR (low);
-	  if (! decode_secs_only)
+	  if (cform != CFORM_SECS_ONLY)
 	    {
 	      if (CONSP (low_tail))
 		{
@@ -877,27 +904,31 @@ decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
 	form = TIMEFORM_INVALID;
     }
   else if (FASTER_TIMEFNS && INTEGERP (specified_time))
-    {
-      decode_ticks_hz (specified_time, make_fixnum (1), result, dresult);
-      return form;
-    }
+    return (struct form_time)
+      {
+	.form = form,
+	.time = decode_ticks_hz (specified_time, make_fixnum (1), cform)
+      };
   else if (FLOATP (specified_time))
     {
       double d = XFLOAT_DATA (specified_time);
       if (!isfinite (d))
 	time_error (isnan (d) ? EDOM : EOVERFLOW);
-      if (result)
-	decode_float_time (d, result);
-      else
-	*dresult = d;
-      return TIMEFORM_FLOAT;
+      return (struct form_time)
+	{
+	  .form = TIMEFORM_FLOAT,
+	  .time
+	    = (cform == CFORM_DOUBLE
+	       ? (union c_time) { .d = d }
+	       : (union c_time) { .lt = decode_float_time (d) })
+	};
     }
 
-  int err = decode_time_components (form, high, low, usec, psec,
-				    result, dresult);
-  if (err)
-    time_error (err);
-  return form;
+  struct err_time err_time
+    = decode_time_components (form, high, low, usec, psec, cform);
+  if (err_time.err)
+    time_error (err_time.err);
+  return (struct form_time) { .form = form, .time = err_time.time };
 }
 
 /* Convert a non-float Lisp timestamp SPECIFIED_TIME to double.
@@ -905,9 +936,7 @@ decode_lisp_time (Lisp_Object specified_time, bool decode_secs_only,
 double
 float_time (Lisp_Object specified_time)
 {
-  double t;
-  decode_lisp_time (specified_time, false, 0, &t);
-  return t;
+  return decode_lisp_time (specified_time, CFORM_DOUBLE).time.d;
 }
 
 /* Convert Z to time_t, returning true if it fits.  */
@@ -1000,32 +1029,26 @@ lisp_to_timespec (struct lisp_time t)
 }
 
 /* Convert (HIGH LOW USEC PSEC) to struct timespec.
-   Return true if successful.  */
-bool
+   Return a valid timestamp if successful, an invalid one otherwise.  */
+struct timespec
 list4_to_timespec (Lisp_Object high, Lisp_Object low,
-		   Lisp_Object usec, Lisp_Object psec,
-		   struct timespec *result)
+		   Lisp_Object usec, Lisp_Object psec)
 {
-  struct lisp_time t;
-  if (decode_time_components (TIMEFORM_HI_LO_US_PS, high, low, usec, psec,
-			      &t, 0))
-    return false;
-  *result = lisp_to_timespec (t);
-  return timespec_valid_p (*result);
+  struct err_time err_time
+    = decode_time_components (TIMEFORM_HI_LO_US_PS, high, low, usec, psec,
+			      CFORM_TICKS_HZ);
+  return (err_time.err
+	  ? invalid_timespec ()
+	  : lisp_to_timespec (err_time.time.lt));
 }
 
 /* Decode a Lisp list SPECIFIED_TIME that represents a time.
    If SPECIFIED_TIME is nil, use the current time.
-   Signal an error if SPECIFIED_TIME does not represent a time.
-   If PFORM, store the time's form into *PFORM.  */
+   Signal an error if SPECIFIED_TIME does not represent a time.  */
 static struct lisp_time
-lisp_time_struct (Lisp_Object specified_time, enum timeform *pform)
+lisp_time_struct (Lisp_Object specified_time)
 {
-  struct lisp_time t;
-  enum timeform form = decode_lisp_time (specified_time, false, &t, 0);
-  if (pform)
-    *pform = form;
-  return t;
+  return decode_lisp_time (specified_time, CFORM_TICKS_HZ).time.lt;
 }
 
 /* Decode a Lisp list SPECIFIED_TIME that represents a time.
@@ -1035,7 +1058,7 @@ lisp_time_struct (Lisp_Object specified_time, enum timeform *pform)
 struct timespec
 lisp_time_argument (Lisp_Object specified_time)
 {
-  struct lisp_time lt = lisp_time_struct (specified_time, 0);
+  struct lisp_time lt = lisp_time_struct (specified_time);
   struct timespec t = lisp_to_timespec (lt);
   if (! timespec_valid_p (t))
     time_overflow ();
@@ -1047,9 +1070,8 @@ lisp_time_argument (Lisp_Object specified_time)
 static time_t
 lisp_seconds_argument (Lisp_Object specified_time)
 {
-  struct lisp_time lt;
-  decode_lisp_time (specified_time, true, &lt, 0);
-  struct timespec t = lisp_to_timespec (lt);
+  struct form_time ft = decode_lisp_time (specified_time, CFORM_SECS_ONLY);
+  struct timespec t = lisp_to_timespec (ft.time.lt);
   if (! timespec_valid_p (t))
     time_overflow ();
   return t.tv_sec;
@@ -1096,9 +1118,11 @@ lispint_arith (Lisp_Object a, Lisp_Object b, bool subtract)
 static Lisp_Object
 time_arith (Lisp_Object a, Lisp_Object b, bool subtract)
 {
-  enum timeform aform, bform;
-  struct lisp_time ta = lisp_time_struct (a, &aform);
-  struct lisp_time tb = lisp_time_struct (b, &bform);
+  struct form_time
+    fta = decode_lisp_time (a, CFORM_TICKS_HZ),
+    ftb = decode_lisp_time (b, CFORM_TICKS_HZ);
+  enum timeform aform = fta.form, bform = ftb.form;
+  struct lisp_time ta = fta.time.lt, tb = ftb.time.lt;
   Lisp_Object ticks, hz;
 
   if (FASTER_TIMEFNS && BASE_EQ (ta.hz, tb.hz))
@@ -1239,8 +1263,8 @@ time_cmp (Lisp_Object a, Lisp_Object b)
 
   /* Compare (ATICKS . AZ) to (BTICKS . BHZ) by comparing
      ATICKS * BHZ to BTICKS * AHZ.  */
-  struct lisp_time ta = lisp_time_struct (a, 0);
-  struct lisp_time tb = lisp_time_struct (b, 0);
+  struct lisp_time ta = lisp_time_struct (a);
+  struct lisp_time tb = lisp_time_struct (b);
   mpz_t const *za = bignum_integer (&mpz[0], ta.ticks);
   mpz_t const *zb = bignum_integer (&mpz[1], tb.ticks);
   if (! (FASTER_TIMEFNS && BASE_EQ (ta.hz, tb.hz)))
@@ -1517,7 +1541,7 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
   (Lisp_Object specified_time, Lisp_Object zone, Lisp_Object form)
 {
   /* Compute broken-down local time LOCAL_TM from SPECIFIED_TIME and ZONE.  */
-  struct lisp_time lt = lisp_time_struct (specified_time, 0);
+  struct lisp_time lt = lisp_time_struct (specified_time);
   struct timespec ts = lisp_to_timespec (lt);
   if (! timespec_valid_p (ts))
     time_overflow ();
@@ -1695,8 +1719,7 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0,
     }
 
   /* Let SEC = floor (LT.ticks / HZ), with SUBSECTICKS the remainder.  */
-  struct lisp_time lt;
-  decode_lisp_time (secarg, false, &lt, 0);
+  struct lisp_time lt = decode_lisp_time (secarg, CFORM_TICKS_HZ).time.lt;
   Lisp_Object hz = lt.hz, sec, subsecticks;
   if (FASTER_TIMEFNS && BASE_EQ (hz, make_fixnum (1)))
     {
@@ -1765,8 +1788,8 @@ DEFUN ("time-convert", Ftime_convert, Stime_convert, 1, 2, 0,
 {
   /* FIXME: Any reason why we don't offer a `float` output format option as
      well, since we accept it as input?  */
-  struct lisp_time t;
-  enum timeform input_form = decode_lisp_time (time, false, &t, 0);
+  struct form_time form_time = decode_lisp_time (time, CFORM_TICKS_HZ);
+  struct lisp_time t = form_time.time.lt;
   form = (!NILP (form) ? maybe_remove_pos_from_symbol (form)
 	  : current_time_list ? Qlist : Qt);
   if (BASE_EQ (form, Qlist))
@@ -1776,7 +1799,7 @@ DEFUN ("time-convert", Ftime_convert, Stime_convert, 1, 2, 0,
   if (BASE_EQ (form, Qt))
     form = t.hz;
   if (FASTER_TIMEFNS
-      && input_form == TIMEFORM_TICKS_HZ && BASE_EQ (form, XCDR (time)))
+      && form_time.form == TIMEFORM_TICKS_HZ && BASE_EQ (form, XCDR (time)))
     return time;
   return Fcons (lisp_time_hz_ticks (t, form), form);
 }
-- 
2.34.1


[-- Attachment #3: 0002-Refactor-timefns-order.patch --]
[-- Type: text/x-patch, Size: 14802 bytes --]

From 7d0a769e2a275edd0b7215242170d5e4b53e668e Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Jul 2024 15:42:10 +0200
Subject: [PATCH 02/17] Refactor timefns order

Move definitions around in timefns.c.  This does not affect the
implementation; it merely makes future changes easier to follow.
* src/timefns.c (frac_to_double, mpz_time, lisp_to_timespec)
(enum cform, union c_time, decode_ticks_hz): Move earlier.
---
 src/timefns.c | 400 +++++++++++++++++++++++++-------------------------
 1 file changed, 200 insertions(+), 200 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 70961c1a560..a7a7d552506 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -400,6 +400,112 @@ lo_time (time_t t)
    equals FLT_RADIX**P.  */
 static Lisp_Object flt_radix_power;
 
+/* Return NUMERATOR / DENOMINATOR, rounded to the nearest double.
+   Arguments must be Lisp integers, and DENOMINATOR must be positive.  */
+static double
+frac_to_double (Lisp_Object numerator, Lisp_Object denominator)
+{
+  intmax_t intmax_numerator, intmax_denominator;
+  if (FASTER_TIMEFNS
+      && integer_to_intmax (numerator, &intmax_numerator)
+      && integer_to_intmax (denominator, &intmax_denominator)
+      && intmax_numerator % intmax_denominator == 0)
+    return intmax_numerator / intmax_denominator;
+
+  /* Compute number of base-FLT_RADIX digits in numerator and denominator.  */
+  mpz_t const *n = bignum_integer (&mpz[0], numerator);
+  mpz_t const *d = bignum_integer (&mpz[1], denominator);
+  ptrdiff_t ndig = mpz_sizeinbase (*n, FLT_RADIX);
+  ptrdiff_t ddig = mpz_sizeinbase (*d, FLT_RADIX);
+
+  /* Scale with SCALE when doing integer division.  That is, compute
+     (N * FLT_RADIX**SCALE) / D [or, if SCALE is negative, N / (D *
+     FLT_RADIX**-SCALE)] as a bignum, convert the bignum to double,
+     then divide the double by FLT_RADIX**SCALE.  First scale N
+     (or scale D, if SCALE is negative) ...  */
+  ptrdiff_t scale = ddig - ndig + DBL_MANT_DIG;
+  if (scale < 0)
+    {
+      mpz_mul_2exp (mpz[1], *d, - (scale * LOG2_FLT_RADIX));
+      d = &mpz[1];
+    }
+  else
+    {
+      /* min so we don't scale tiny numbers as if they were normalized.  */
+      scale = min (scale, flt_radix_power_size - 1);
+
+      mpz_mul_2exp (mpz[0], *n, scale * LOG2_FLT_RADIX);
+      n = &mpz[0];
+    }
+  /* ... and then divide, with quotient Q and remainder R.  */
+  mpz_t *q = &mpz[2];
+  mpz_t *r = &mpz[3];
+  mpz_tdiv_qr (*q, *r, *n, *d);
+
+  /* The amount to add to the absolute value of Q so that truncating
+     it to double will round correctly.  */
+  int incr;
+
+  /* Round the quotient before converting it to double.
+     If the quotient is less than FLT_RADIX ** DBL_MANT_DIG,
+     round to the nearest integer; otherwise, it is less than
+     FLT_RADIX ** (DBL_MANT_DIG + 1) and round it to the nearest
+     multiple of FLT_RADIX.  Break ties to even.  */
+  if (mpz_sizeinbase (*q, FLT_RADIX) <= DBL_MANT_DIG)
+    {
+      /* Converting to double will use the whole quotient so add 1 to
+	 its absolute value as per round-to-even; i.e., if the doubled
+	 remainder exceeds the denominator, or exactly equals the
+	 denominator and adding 1 would make the quotient even.  */
+      mpz_mul_2exp (*r, *r, 1);
+      int cmp = mpz_cmpabs (*r, *d);
+      incr = cmp > 0 || (cmp == 0 && (FASTER_TIMEFNS && FLT_RADIX == 2
+				      ? mpz_odd_p (*q)
+				      : mpz_tdiv_ui (*q, FLT_RADIX) & 1));
+    }
+  else
+    {
+      /* Converting to double will discard the quotient's low-order digit,
+	 so add FLT_RADIX to its absolute value as per round-to-even.  */
+      int lo_2digits = mpz_tdiv_ui (*q, FLT_RADIX * FLT_RADIX);
+      eassume (0 <= lo_2digits && lo_2digits < FLT_RADIX * FLT_RADIX);
+      int lo_digit = lo_2digits % FLT_RADIX;
+      incr = ((lo_digit > FLT_RADIX / 2
+	       || (lo_digit == FLT_RADIX / 2 && FLT_RADIX % 2 == 0
+		   && ((lo_2digits / FLT_RADIX) & 1
+		       || mpz_sgn (*r) != 0)))
+	      ? FLT_RADIX : 0);
+    }
+
+  /* Increment the absolute value of the quotient by INCR.  */
+  if (!FASTER_TIMEFNS || incr != 0)
+    (mpz_sgn (*n) < 0 ? mpz_sub_ui : mpz_add_ui) (*q, *q, incr);
+
+  /* Rescale the integer Q back to double.  This step does not round.  */
+  return scalbn (mpz_get_d (*q), -scale);
+}
+
+/* Convert Z to time_t, returning true if it fits.  */
+static bool
+mpz_time (mpz_t const z, time_t *t)
+{
+  if (TYPE_SIGNED (time_t))
+    {
+      intmax_t i;
+      if (! (mpz_to_intmax (z, &i) && TIME_T_MIN <= i && i <= TIME_T_MAX))
+	return false;
+      *t = i;
+    }
+  else
+    {
+      uintmax_t i;
+      if (! (mpz_to_uintmax (z, &i) && i <= TIME_T_MAX))
+	return false;
+      *t = i;
+    }
+  return true;
+}
+
 /* Components of a Lisp timestamp (TICKS . HZ).  Using this C struct can
    avoid the consing overhead of creating (TICKS . HZ).  */
 struct lisp_time
@@ -411,6 +517,100 @@ lo_time (time_t t)
   Lisp_Object hz;
 };
 
+/* Convert T to struct timespec, returning an invalid timespec
+   if T does not fit.  */
+static struct timespec
+lisp_to_timespec (struct lisp_time t)
+{
+  struct timespec result = invalid_timespec ();
+  int ns;
+  mpz_t *q = &mpz[0];
+  mpz_t const *qt = q;
+
+  /* Floor-divide (T.ticks * TIMESPEC_HZ) by T.hz,
+     yielding quotient Q (tv_sec) and remainder NS (tv_nsec).
+     Return an invalid timespec if Q does not fit in time_t.
+     For speed, prefer fixnum arithmetic if it works.  */
+  if (FASTER_TIMEFNS && BASE_EQ (t.hz, timespec_hz))
+    {
+      if (FIXNUMP (t.ticks))
+	{
+	  EMACS_INT s = XFIXNUM (t.ticks) / TIMESPEC_HZ;
+	  ns = XFIXNUM (t.ticks) % TIMESPEC_HZ;
+	  if (ns < 0)
+	    s--, ns += TIMESPEC_HZ;
+	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
+	      && s <= TIME_T_MAX)
+	    {
+	      result.tv_sec = s;
+	      result.tv_nsec = ns;
+	    }
+	  return result;
+	}
+      else
+	ns = mpz_fdiv_q_ui (*q, *xbignum_val (t.ticks), TIMESPEC_HZ);
+    }
+  else if (FASTER_TIMEFNS && BASE_EQ (t.hz, make_fixnum (1)))
+    {
+      ns = 0;
+      if (FIXNUMP (t.ticks))
+	{
+	  EMACS_INT s = XFIXNUM (t.ticks);
+	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
+	      && s <= TIME_T_MAX)
+	    {
+	      result.tv_sec = s;
+	      result.tv_nsec = ns;
+	    }
+	  return result;
+	}
+      else
+	qt = xbignum_val (t.ticks);
+    }
+  else
+    {
+      mpz_mul_ui (*q, *bignum_integer (q, t.ticks), TIMESPEC_HZ);
+      mpz_fdiv_q (*q, *q, *bignum_integer (&mpz[1], t.hz));
+      ns = mpz_fdiv_q_ui (*q, *q, TIMESPEC_HZ);
+    }
+
+  /* Check that Q fits in time_t, not merely in T.tv_sec.  With some versions
+     of MinGW, tv_sec is a 64-bit type, whereas time_t is a 32-bit type.  */
+  time_t sec;
+  if (mpz_time (*qt, &sec))
+    {
+      result.tv_sec = sec;
+      result.tv_nsec = ns;
+    }
+  return result;
+}
+
+/* C timestamp forms.  This enum is passed to conversion functions to
+   specify the desired C timestamp form.  */
+enum cform
+  {
+    CFORM_TICKS_HZ, /* struct lisp_time */
+    CFORM_SECS_ONLY, /* struct lisp_time but HZ is 1 */
+    CFORM_DOUBLE /* double */
+  };
+
+/* A C timestamp in one of the forms specified by enum cform.  */
+union c_time
+{
+  struct lisp_time lt;
+  double d;
+};
+
+/* From a valid timestamp (TICKS . HZ), generate the corresponding
+   time value in CFORM form.  */
+static union c_time
+decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz, enum cform cform)
+{
+  return (cform == CFORM_DOUBLE
+	  ? (union c_time) { .d = frac_to_double (ticks, hz) }
+	  : (union c_time) { .lt = { .ticks = ticks, .hz = hz } });
+}
+
 /* Convert the finite number T into an Emacs time, truncating
    toward minus infinity.  Signal an error if unsuccessful.  */
 static struct lisp_time
@@ -613,117 +813,6 @@ timespec_to_lisp (struct timespec t)
   return Fcons (timespec_ticks (t), timespec_hz);
 }
 
-/* Return NUMERATOR / DENOMINATOR, rounded to the nearest double.
-   Arguments must be Lisp integers, and DENOMINATOR must be positive.  */
-static double
-frac_to_double (Lisp_Object numerator, Lisp_Object denominator)
-{
-  intmax_t intmax_numerator, intmax_denominator;
-  if (FASTER_TIMEFNS
-      && integer_to_intmax (numerator, &intmax_numerator)
-      && integer_to_intmax (denominator, &intmax_denominator)
-      && intmax_numerator % intmax_denominator == 0)
-    return intmax_numerator / intmax_denominator;
-
-  /* Compute number of base-FLT_RADIX digits in numerator and denominator.  */
-  mpz_t const *n = bignum_integer (&mpz[0], numerator);
-  mpz_t const *d = bignum_integer (&mpz[1], denominator);
-  ptrdiff_t ndig = mpz_sizeinbase (*n, FLT_RADIX);
-  ptrdiff_t ddig = mpz_sizeinbase (*d, FLT_RADIX);
-
-  /* Scale with SCALE when doing integer division.  That is, compute
-     (N * FLT_RADIX**SCALE) / D [or, if SCALE is negative, N / (D *
-     FLT_RADIX**-SCALE)] as a bignum, convert the bignum to double,
-     then divide the double by FLT_RADIX**SCALE.  First scale N
-     (or scale D, if SCALE is negative) ...  */
-  ptrdiff_t scale = ddig - ndig + DBL_MANT_DIG;
-  if (scale < 0)
-    {
-      mpz_mul_2exp (mpz[1], *d, - (scale * LOG2_FLT_RADIX));
-      d = &mpz[1];
-    }
-  else
-    {
-      /* min so we don't scale tiny numbers as if they were normalized.  */
-      scale = min (scale, flt_radix_power_size - 1);
-
-      mpz_mul_2exp (mpz[0], *n, scale * LOG2_FLT_RADIX);
-      n = &mpz[0];
-    }
-  /* ... and then divide, with quotient Q and remainder R.  */
-  mpz_t *q = &mpz[2];
-  mpz_t *r = &mpz[3];
-  mpz_tdiv_qr (*q, *r, *n, *d);
-
-  /* The amount to add to the absolute value of Q so that truncating
-     it to double will round correctly.  */
-  int incr;
-
-  /* Round the quotient before converting it to double.
-     If the quotient is less than FLT_RADIX ** DBL_MANT_DIG,
-     round to the nearest integer; otherwise, it is less than
-     FLT_RADIX ** (DBL_MANT_DIG + 1) and round it to the nearest
-     multiple of FLT_RADIX.  Break ties to even.  */
-  if (mpz_sizeinbase (*q, FLT_RADIX) <= DBL_MANT_DIG)
-    {
-      /* Converting to double will use the whole quotient so add 1 to
-	 its absolute value as per round-to-even; i.e., if the doubled
-	 remainder exceeds the denominator, or exactly equals the
-	 denominator and adding 1 would make the quotient even.  */
-      mpz_mul_2exp (*r, *r, 1);
-      int cmp = mpz_cmpabs (*r, *d);
-      incr = cmp > 0 || (cmp == 0 && (FASTER_TIMEFNS && FLT_RADIX == 2
-				      ? mpz_odd_p (*q)
-				      : mpz_tdiv_ui (*q, FLT_RADIX) & 1));
-    }
-  else
-    {
-      /* Converting to double will discard the quotient's low-order digit,
-	 so add FLT_RADIX to its absolute value as per round-to-even.  */
-      int lo_2digits = mpz_tdiv_ui (*q, FLT_RADIX * FLT_RADIX);
-      eassume (0 <= lo_2digits && lo_2digits < FLT_RADIX * FLT_RADIX);
-      int lo_digit = lo_2digits % FLT_RADIX;
-      incr = ((lo_digit > FLT_RADIX / 2
-	       || (lo_digit == FLT_RADIX / 2 && FLT_RADIX % 2 == 0
-		   && ((lo_2digits / FLT_RADIX) & 1
-		       || mpz_sgn (*r) != 0)))
-	      ? FLT_RADIX : 0);
-    }
-
-  /* Increment the absolute value of the quotient by INCR.  */
-  if (!FASTER_TIMEFNS || incr != 0)
-    (mpz_sgn (*n) < 0 ? mpz_sub_ui : mpz_add_ui) (*q, *q, incr);
-
-  /* Rescale the integer Q back to double.  This step does not round.  */
-  return scalbn (mpz_get_d (*q), -scale);
-}
-
-/* C timestamp forms.  This enum is passed to conversion functions to
-   specify the desired C timestamp form.  */
-enum cform
-  {
-    CFORM_TICKS_HZ, /* struct lisp_time */
-    CFORM_SECS_ONLY, /* struct lisp_time but HZ is 1 */
-    CFORM_DOUBLE /* double */
-  };
-
-/* A C timestamp in one of the forms specified by enum cform.  */
-union c_time
-{
-  struct lisp_time lt;
-  double d;
-};
-
-/* From a valid timestamp (TICKS . HZ), generate the corresponding
-   time value in CFORM form.  */
-static union c_time
-decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz, enum cform cform)
-{
-  return (cform == CFORM_DOUBLE
-	  ? (union c_time) { .d = frac_to_double (ticks, hz) }
-	  : (union c_time) { .lt = { .ticks = ticks, .hz = hz } });
-}
-
 /* An (error number, C timestamp) pair.  */
 struct err_time
 {
@@ -939,95 +1028,6 @@ float_time (Lisp_Object specified_time)
   return decode_lisp_time (specified_time, CFORM_DOUBLE).time.d;
 }
 
-/* Convert Z to time_t, returning true if it fits.  */
-static bool
-mpz_time (mpz_t const z, time_t *t)
-{
-  if (TYPE_SIGNED (time_t))
-    {
-      intmax_t i;
-      if (! (mpz_to_intmax (z, &i) && TIME_T_MIN <= i && i <= TIME_T_MAX))
-	return false;
-      *t = i;
-    }
-  else
-    {
-      uintmax_t i;
-      if (! (mpz_to_uintmax (z, &i) && i <= TIME_T_MAX))
-	return false;
-      *t = i;
-    }
-  return true;
-}
-
-/* Convert T to struct timespec, returning an invalid timespec
-   if T does not fit.  */
-static struct timespec
-lisp_to_timespec (struct lisp_time t)
-{
-  struct timespec result = invalid_timespec ();
-  int ns;
-  mpz_t *q = &mpz[0];
-  mpz_t const *qt = q;
-
-  /* Floor-divide (T.ticks * TIMESPEC_HZ) by T.hz,
-     yielding quotient Q (tv_sec) and remainder NS (tv_nsec).
-     Return an invalid timespec if Q does not fit in time_t.
-     For speed, prefer fixnum arithmetic if it works.  */
-  if (FASTER_TIMEFNS && BASE_EQ (t.hz, timespec_hz))
-    {
-      if (FIXNUMP (t.ticks))
-	{
-	  EMACS_INT s = XFIXNUM (t.ticks) / TIMESPEC_HZ;
-	  ns = XFIXNUM (t.ticks) % TIMESPEC_HZ;
-	  if (ns < 0)
-	    s--, ns += TIMESPEC_HZ;
-	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
-	      && s <= TIME_T_MAX)
-	    {
-	      result.tv_sec = s;
-	      result.tv_nsec = ns;
-	    }
-	  return result;
-	}
-      else
-	ns = mpz_fdiv_q_ui (*q, *xbignum_val (t.ticks), TIMESPEC_HZ);
-    }
-  else if (FASTER_TIMEFNS && BASE_EQ (t.hz, make_fixnum (1)))
-    {
-      ns = 0;
-      if (FIXNUMP (t.ticks))
-	{
-	  EMACS_INT s = XFIXNUM (t.ticks);
-	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
-	      && s <= TIME_T_MAX)
-	    {
-	      result.tv_sec = s;
-	      result.tv_nsec = ns;
-	    }
-	  return result;
-	}
-      else
-	qt = xbignum_val (t.ticks);
-    }
-  else
-    {
-      mpz_mul_ui (*q, *bignum_integer (q, t.ticks), TIMESPEC_HZ);
-      mpz_fdiv_q (*q, *q, *bignum_integer (&mpz[1], t.hz));
-      ns = mpz_fdiv_q_ui (*q, *q, TIMESPEC_HZ);
-    }
-
-  /* Check that Q fits in time_t, not merely in T.tv_sec.  With some versions
-     of MinGW, tv_sec is a 64-bit type, whereas time_t is a 32-bit type.  */
-  time_t sec;
-  if (mpz_time (*qt, &sec))
-    {
-      result.tv_sec = sec;
-      result.tv_nsec = ns;
-    }
-  return result;
-}
-
 /* Convert (HIGH LOW USEC PSEC) to struct timespec.
    Return a valid timestamp if successful, an invalid one otherwise.  */
 struct timespec
-- 
2.34.1


[-- Attachment #4: 0003-Refactor-decode_ticks_hz-via-switch.patch --]
[-- Type: text/x-patch, Size: 1190 bytes --]

From a79b25b987f0288bd0e773ccb9f2b4a49595898d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Jul 2024 16:05:52 +0200
Subject: [PATCH 03/17] Refactor decode_ticks_hz via switch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/timefns.c (decode_ticks_hz): Change ?: to ‘switch’,
for benefit of future changes.
---
 src/timefns.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index a7a7d552506..ac41a3d6958 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -606,9 +606,14 @@ lisp_to_timespec (struct lisp_time t)
 static union c_time
 decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz, enum cform cform)
 {
-  return (cform == CFORM_DOUBLE
-	  ? (union c_time) { .d = frac_to_double (ticks, hz) }
-	  : (union c_time) { .lt = { .ticks = ticks, .hz = hz } });
+  switch (cform)
+    {
+    case CFORM_DOUBLE:
+      return (union c_time) { .d = frac_to_double (ticks, hz) };
+
+    default:
+      return (union c_time) { .lt = { .ticks = ticks, .hz = hz } };
+    }
 }
 
 /* Convert the finite number T into an Emacs time, truncating
-- 
2.34.1


[-- Attachment #5: 0004-Split-lisp_to_timespec-in-two.patch --]
[-- Type: text/x-patch, Size: 3776 bytes --]

From cca50c17f4edf11e4dde484d0cfc62b740b8163a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Jul 2024 16:18:18 +0200
Subject: [PATCH 04/17] Split lisp_to_timespec in two

* src/timefns.c (ticks_hz_to_timespec): New function,
which is almost all the old lisp_to_timespec but with
a 2-arg API.  This should help further changes.
(lisp_to_timespec): Use it.
---
 src/timefns.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index ac41a3d6958..c748867b54d 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -517,26 +517,26 @@ mpz_time (mpz_t const z, time_t *t)
   Lisp_Object hz;
 };
 
-/* Convert T to struct timespec, returning an invalid timespec
-   if T does not fit.  */
+/* Convert (TICKS . HZ) to struct timespec, returning an invalid
+   timespec if the result would not fit.  */
 static struct timespec
-lisp_to_timespec (struct lisp_time t)
+ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
 {
   struct timespec result = invalid_timespec ();
   int ns;
   mpz_t *q = &mpz[0];
   mpz_t const *qt = q;
 
-  /* Floor-divide (T.ticks * TIMESPEC_HZ) by T.hz,
+  /* Floor-divide (TICKS * TIMESPEC_HZ) by HZ,
      yielding quotient Q (tv_sec) and remainder NS (tv_nsec).
      Return an invalid timespec if Q does not fit in time_t.
      For speed, prefer fixnum arithmetic if it works.  */
-  if (FASTER_TIMEFNS && BASE_EQ (t.hz, timespec_hz))
+  if (FASTER_TIMEFNS && BASE_EQ (hz, timespec_hz))
     {
-      if (FIXNUMP (t.ticks))
+      if (FIXNUMP (ticks))
 	{
-	  EMACS_INT s = XFIXNUM (t.ticks) / TIMESPEC_HZ;
-	  ns = XFIXNUM (t.ticks) % TIMESPEC_HZ;
+	  EMACS_INT s = XFIXNUM (ticks) / TIMESPEC_HZ;
+	  ns = XFIXNUM (ticks) % TIMESPEC_HZ;
 	  if (ns < 0)
 	    s--, ns += TIMESPEC_HZ;
 	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
@@ -548,14 +548,14 @@ lisp_to_timespec (struct lisp_time t)
 	  return result;
 	}
       else
-	ns = mpz_fdiv_q_ui (*q, *xbignum_val (t.ticks), TIMESPEC_HZ);
+	ns = mpz_fdiv_q_ui (*q, *xbignum_val (ticks), TIMESPEC_HZ);
     }
-  else if (FASTER_TIMEFNS && BASE_EQ (t.hz, make_fixnum (1)))
+  else if (FASTER_TIMEFNS && BASE_EQ (hz, make_fixnum (1)))
     {
       ns = 0;
-      if (FIXNUMP (t.ticks))
+      if (FIXNUMP (ticks))
 	{
-	  EMACS_INT s = XFIXNUM (t.ticks);
+	  EMACS_INT s = XFIXNUM (ticks);
 	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
 	      && s <= TIME_T_MAX)
 	    {
@@ -565,17 +565,17 @@ lisp_to_timespec (struct lisp_time t)
 	  return result;
 	}
       else
-	qt = xbignum_val (t.ticks);
+	qt = xbignum_val (ticks);
     }
   else
     {
-      mpz_mul_ui (*q, *bignum_integer (q, t.ticks), TIMESPEC_HZ);
-      mpz_fdiv_q (*q, *q, *bignum_integer (&mpz[1], t.hz));
+      mpz_mul_ui (*q, *bignum_integer (q, ticks), TIMESPEC_HZ);
+      mpz_fdiv_q (*q, *q, *bignum_integer (&mpz[1], hz));
       ns = mpz_fdiv_q_ui (*q, *q, TIMESPEC_HZ);
     }
 
-  /* Check that Q fits in time_t, not merely in T.tv_sec.  With some versions
-     of MinGW, tv_sec is a 64-bit type, whereas time_t is a 32-bit type.  */
+  /* Check that Q fits in time_t, not merely in RESULT.tv_sec.  With some MinGW
+     versions, tv_sec is a 64-bit type, whereas time_t is a 32-bit type.  */
   time_t sec;
   if (mpz_time (*qt, &sec))
     {
@@ -585,6 +585,14 @@ lisp_to_timespec (struct lisp_time t)
   return result;
 }
 
+/* Convert T to struct timespec, returning an invalid timespec
+   if T does not fit.  */
+static struct timespec
+lisp_to_timespec (struct lisp_time t)
+{
+  return ticks_hz_to_timespec (t.ticks, t.hz);
+}
+
 /* C timestamp forms.  This enum is passed to conversion functions to
    specify the desired C timestamp form.  */
 enum cform
-- 
2.34.1


[-- Attachment #6: 0005-Push-some-time-conversions-down.patch --]
[-- Type: text/x-patch, Size: 6483 bytes --]

From faf78003dfb310b2d381b1748c57be568632ca8b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Jul 2024 20:50:47 +0200
Subject: [PATCH 05/17] Push some time conversions down

* src/timefns.c: Push some time conversions down to lower level fns.
This is a win in its own right and should allow for further speedups.
(lisp_to_timespec): Remove; this convenience function is no longer
needed now that there would be only one caller.  Remaining caller
changed to use definiens.
(enum cform): New constant CFORM_TIMESPEC.  Also, CFORM_SECS_ONLY
now generates a struct timespec instead of a struct lisp_time.
(union c_time.ts): New member.
(decode_ticks_hz): Handle new struct timespec cases.
(decode_float_time, lisp_time_struct): New arg cform, and return
union c_time rather than struct lisp_time.  All callers changed.
(list4_to_timespec, lisp_time_argument, lisp_seconds_argument):
Let lower-level function do the conversion, to allow for better
optimization.
---
 src/timefns.c | 61 +++++++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index c748867b54d..cc148fa9752 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -585,20 +585,13 @@ ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
   return result;
 }
 
-/* Convert T to struct timespec, returning an invalid timespec
-   if T does not fit.  */
-static struct timespec
-lisp_to_timespec (struct lisp_time t)
-{
-  return ticks_hz_to_timespec (t.ticks, t.hz);
-}
-
 /* C timestamp forms.  This enum is passed to conversion functions to
    specify the desired C timestamp form.  */
 enum cform
   {
     CFORM_TICKS_HZ, /* struct lisp_time */
-    CFORM_SECS_ONLY, /* struct lisp_time but HZ is 1 */
+    CFORM_TIMESPEC, /* struct timespec */
+    CFORM_SECS_ONLY, /* struct timespec but tv_nsec == 0 if timespec valid */
     CFORM_DOUBLE /* double */
   };
 
@@ -606,6 +599,7 @@ lisp_to_timespec (struct lisp_time t)
 union c_time
 {
   struct lisp_time lt;
+  struct timespec ts;
   double d;
 };
 
@@ -619,16 +613,22 @@ decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz, enum cform cform)
     case CFORM_DOUBLE:
       return (union c_time) { .d = frac_to_double (ticks, hz) };
 
-    default:
+    case CFORM_TICKS_HZ:
       return (union c_time) { .lt = { .ticks = ticks, .hz = hz } };
+
+    default:
+      return (union c_time) { .ts = ticks_hz_to_timespec (ticks, hz) };
     }
 }
 
-/* Convert the finite number T into an Emacs time, truncating
+/* Convert the finite number T into a C time of form CFORM, truncating
    toward minus infinity.  Signal an error if unsuccessful.  */
-static struct lisp_time
-decode_float_time (double t)
+static union c_time
+decode_float_time (double t, enum cform cform)
 {
+  if (FASTER_TIMEFNS && cform == CFORM_DOUBLE)
+    return (union c_time) { .d = t };
+
   Lisp_Object ticks, hz;
   if (t == 0)
     {
@@ -671,7 +671,7 @@ decode_float_time (double t)
 	  ASET (flt_radix_power, scale, hz);
 	}
     }
-  return (struct lisp_time) { .ticks = ticks, .hz = hz };
+  return decode_ticks_hz (ticks, hz, cform);
 }
 
 /* Make a 4-element timestamp (HI LO US PS) from TICKS and HZ.
@@ -1019,10 +1019,7 @@ decode_lisp_time (Lisp_Object specified_time, enum cform cform)
       return (struct form_time)
 	{
 	  .form = TIMEFORM_FLOAT,
-	  .time
-	    = (cform == CFORM_DOUBLE
-	       ? (union c_time) { .d = d }
-	       : (union c_time) { .lt = decode_float_time (d) })
+	  .time = decode_float_time (d, cform)
 	};
     }
 
@@ -1049,19 +1046,18 @@ list4_to_timespec (Lisp_Object high, Lisp_Object low,
 {
   struct err_time err_time
     = decode_time_components (TIMEFORM_HI_LO_US_PS, high, low, usec, psec,
-			      CFORM_TICKS_HZ);
-  return (err_time.err
-	  ? invalid_timespec ()
-	  : lisp_to_timespec (err_time.time.lt));
+			      CFORM_TIMESPEC);
+  return err_time.err ? invalid_timespec () : err_time.time.ts;
 }
 
 /* Decode a Lisp list SPECIFIED_TIME that represents a time.
    If SPECIFIED_TIME is nil, use the current time.
+   Decode to CFORM form.
    Signal an error if SPECIFIED_TIME does not represent a time.  */
-static struct lisp_time
-lisp_time_struct (Lisp_Object specified_time)
+static union c_time
+lisp_time_struct (Lisp_Object specified_time, enum cform cform)
 {
-  return decode_lisp_time (specified_time, CFORM_TICKS_HZ).time.lt;
+  return decode_lisp_time (specified_time, cform).time;
 }
 
 /* Decode a Lisp list SPECIFIED_TIME that represents a time.
@@ -1071,8 +1067,7 @@ lisp_time_struct (Lisp_Object specified_time)
 struct timespec
 lisp_time_argument (Lisp_Object specified_time)
 {
-  struct lisp_time lt = lisp_time_struct (specified_time);
-  struct timespec t = lisp_to_timespec (lt);
+  struct timespec t = lisp_time_struct (specified_time, CFORM_TIMESPEC).ts;
   if (! timespec_valid_p (t))
     time_overflow ();
   return t;
@@ -1083,8 +1078,8 @@ lisp_time_argument (Lisp_Object specified_time)
 static time_t
 lisp_seconds_argument (Lisp_Object specified_time)
 {
-  struct form_time ft = decode_lisp_time (specified_time, CFORM_SECS_ONLY);
-  struct timespec t = lisp_to_timespec (ft.time.lt);
+  struct timespec t
+    = decode_lisp_time (specified_time, CFORM_SECS_ONLY).time.ts;
   if (! timespec_valid_p (t))
     time_overflow ();
   return t.tv_sec;
@@ -1276,8 +1271,8 @@ time_cmp (Lisp_Object a, Lisp_Object b)
 
   /* Compare (ATICKS . AZ) to (BTICKS . BHZ) by comparing
      ATICKS * BHZ to BTICKS * AHZ.  */
-  struct lisp_time ta = lisp_time_struct (a);
-  struct lisp_time tb = lisp_time_struct (b);
+  struct lisp_time ta = lisp_time_struct (a, CFORM_TICKS_HZ).lt;
+  struct lisp_time tb = lisp_time_struct (b, CFORM_TICKS_HZ).lt;
   mpz_t const *za = bignum_integer (&mpz[0], ta.ticks);
   mpz_t const *zb = bignum_integer (&mpz[1], tb.ticks);
   if (! (FASTER_TIMEFNS && BASE_EQ (ta.hz, tb.hz)))
@@ -1554,8 +1549,8 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
   (Lisp_Object specified_time, Lisp_Object zone, Lisp_Object form)
 {
   /* Compute broken-down local time LOCAL_TM from SPECIFIED_TIME and ZONE.  */
-  struct lisp_time lt = lisp_time_struct (specified_time);
-  struct timespec ts = lisp_to_timespec (lt);
+  struct lisp_time lt = lisp_time_struct (specified_time, CFORM_TICKS_HZ).lt;
+  struct timespec ts = ticks_hz_to_timespec (lt.ticks, lt.hz);
   if (! timespec_valid_p (ts))
     time_overflow ();
   time_t time_spec = ts.tv_sec;
-- 
2.34.1


[-- Attachment #7: 0006-Speed-up-decode-time-when-not-doing-subseconds.patch --]
[-- Type: text/x-patch, Size: 2206 bytes --]

From 18c38bf827d765e5acc533d64ed90a2bff38ec81 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Jul 2024 21:34:23 +0200
Subject: [PATCH 06/17] Speed up decode-time when not doing subseconds

* src/timefns.c (Fdecode_time): Avoid some unnecessary conversions
in the common case where subsecond resolution is not required.
---
 src/timefns.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index cc148fa9752..ded31997620 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -1548,12 +1548,27 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
 usage: (decode-time &optional TIME ZONE FORM)  */)
   (Lisp_Object specified_time, Lisp_Object zone, Lisp_Object form)
 {
-  /* Compute broken-down local time LOCAL_TM from SPECIFIED_TIME and ZONE.  */
-  struct lisp_time lt = lisp_time_struct (specified_time, CFORM_TICKS_HZ).lt;
-  struct timespec ts = ticks_hz_to_timespec (lt.ticks, lt.hz);
-  if (! timespec_valid_p (ts))
-    time_overflow ();
-  time_t time_spec = ts.tv_sec;
+  /* Convert SPECIFIED_TIME to TIME_SPEC and HZ;
+     if HZ != 1 also set LT.ticks.  */
+  time_t time_spec;
+  Lisp_Object hz;
+  struct lisp_time lt;
+  if (EQ (form, Qt))
+    {
+      lt = lisp_time_struct (specified_time, CFORM_TICKS_HZ).lt;
+      struct timespec ts = ticks_hz_to_timespec (lt.ticks, lt.hz);
+      if (! timespec_valid_p (ts))
+	time_overflow ();
+      time_spec = ts.tv_sec;
+      hz = lt.hz;
+    }
+  else
+    {
+      time_spec = lisp_seconds_argument (specified_time);
+      hz = make_fixnum (1);
+    }
+
+  /* Compute broken-down local time LOCAL_TM from TIME_SPEC and ZONE.  */
   struct tm local_tm, gmt_tm;
   timezone_t tz = tzlookup (zone, false);
   struct tm *tm = emacs_localtime_rz (tz, &time_spec, &local_tm);
@@ -1581,8 +1596,8 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
     }
 
   /* Compute SEC from LOCAL_TM.tm_sec and HZ.  */
-  Lisp_Object hz = lt.hz, sec;
-  if (BASE_EQ (hz, make_fixnum (1)) || !EQ (form, Qt))
+  Lisp_Object sec;
+  if (BASE_EQ (hz, make_fixnum (1)))
     sec = make_fixnum (local_tm.tm_sec);
   else
     {
-- 
2.34.1


[-- Attachment #8: 0007-Rename-timefns-internals.patch --]
[-- Type: text/x-patch, Size: 8806 bytes --]

From 34e96f51ff3ed47845a5d0976f067c6f8e0c37a0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 8 Jul 2024 09:26:14 +0200
Subject: [PATCH 07/17] Rename timefns internals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The old names didn’t fit in the conventions used for newer names.
* src/timefns.c (struct ticks_hz): Rename from struct lisp_time.
(union c_time): Rename lt to th.
(ticks_hz_hz_ticks): Rename from lisp_time_hz_ticks.
(ticks_hz_seconds): Rename from lisp_time_seconds.
All uses changed.
---
 src/timefns.c | 62 +++++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index ded31997620..0c5f3bf3ff1 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -508,7 +508,7 @@ mpz_time (mpz_t const z, time_t *t)
 
 /* Components of a Lisp timestamp (TICKS . HZ).  Using this C struct can
    avoid the consing overhead of creating (TICKS . HZ).  */
-struct lisp_time
+struct ticks_hz
 {
   /* Clock count as a Lisp integer.  */
   Lisp_Object ticks;
@@ -589,7 +589,7 @@ ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
    specify the desired C timestamp form.  */
 enum cform
   {
-    CFORM_TICKS_HZ, /* struct lisp_time */
+    CFORM_TICKS_HZ, /* struct ticks_hz */
     CFORM_TIMESPEC, /* struct timespec */
     CFORM_SECS_ONLY, /* struct timespec but tv_nsec == 0 if timespec valid */
     CFORM_DOUBLE /* double */
@@ -598,7 +598,7 @@ ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
 /* A C timestamp in one of the forms specified by enum cform.  */
 union c_time
 {
-  struct lisp_time lt;
+  struct ticks_hz th;
   struct timespec ts;
   double d;
 };
@@ -614,7 +614,7 @@ decode_ticks_hz (Lisp_Object ticks, Lisp_Object hz, enum cform cform)
       return (union c_time) { .d = frac_to_double (ticks, hz) };
 
     case CFORM_TICKS_HZ:
-      return (union c_time) { .lt = { .ticks = ticks, .hz = hz } };
+      return (union c_time) { .th = { .ticks = ticks, .hz = hz } };
 
     default:
       return (union c_time) { .ts = ticks_hz_to_timespec (ticks, hz) };
@@ -751,7 +751,7 @@ timespec_ticks (struct timespec t)
 /* Convert T to a Lisp integer counting HZ ticks, taking the floor.
    Assume T is valid, but check HZ.  */
 static Lisp_Object
-lisp_time_hz_ticks (struct lisp_time t, Lisp_Object hz)
+ticks_hz_hz_ticks (struct ticks_hz t, Lisp_Object hz)
 {
   /* The idea is to return the floor of ((T.ticks * HZ) / T.hz).  */
 
@@ -785,19 +785,19 @@ lisp_time_hz_ticks (struct lisp_time t, Lisp_Object hz)
 
 /* Convert T to a Lisp integer counting seconds, taking the floor.  */
 static Lisp_Object
-lisp_time_seconds (struct lisp_time t)
+ticks_hz_seconds (struct ticks_hz t)
 {
   /* The idea is to return the floor of T.ticks / T.hz.  */
 
   if (!FASTER_TIMEFNS)
-    return lisp_time_hz_ticks (t, make_fixnum (1));
+    return ticks_hz_hz_ticks (t, make_fixnum (1));
 
   /* For speed, use EMACS_INT arithmetic if it will do.  */
   if (FIXNUMP (t.ticks) && FIXNUMP (t.hz))
     return make_fixnum (XFIXNUM (t.ticks) / XFIXNUM (t.hz)
 			- (XFIXNUM (t.ticks) % XFIXNUM (t.hz) < 0));
 
-  /* For speed, inline what lisp_time_hz_ticks would do.  */
+  /* For speed, inline what ticks_hz_hz_ticks would do.  */
   mpz_fdiv_q (mpz[0],
 	      *bignum_integer (&mpz[0], t.ticks),
 	      *bignum_integer (&mpz[1], t.hz));
@@ -1130,7 +1130,7 @@ time_arith (Lisp_Object a, Lisp_Object b, bool subtract)
     fta = decode_lisp_time (a, CFORM_TICKS_HZ),
     ftb = decode_lisp_time (b, CFORM_TICKS_HZ);
   enum timeform aform = fta.form, bform = ftb.form;
-  struct lisp_time ta = fta.time.lt, tb = ftb.time.lt;
+  struct ticks_hz ta = fta.time.th, tb = ftb.time.th;
   Lisp_Object ticks, hz;
 
   if (FASTER_TIMEFNS && BASE_EQ (ta.hz, tb.hz))
@@ -1271,8 +1271,8 @@ time_cmp (Lisp_Object a, Lisp_Object b)
 
   /* Compare (ATICKS . AZ) to (BTICKS . BHZ) by comparing
      ATICKS * BHZ to BTICKS * AHZ.  */
-  struct lisp_time ta = lisp_time_struct (a, CFORM_TICKS_HZ).lt;
-  struct lisp_time tb = lisp_time_struct (b, CFORM_TICKS_HZ).lt;
+  struct ticks_hz ta = lisp_time_struct (a, CFORM_TICKS_HZ).th;
+  struct ticks_hz tb = lisp_time_struct (b, CFORM_TICKS_HZ).th;
   mpz_t const *za = bignum_integer (&mpz[0], ta.ticks);
   mpz_t const *zb = bignum_integer (&mpz[1], tb.ticks);
   if (! (FASTER_TIMEFNS && BASE_EQ (ta.hz, tb.hz)))
@@ -1549,18 +1549,18 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
   (Lisp_Object specified_time, Lisp_Object zone, Lisp_Object form)
 {
   /* Convert SPECIFIED_TIME to TIME_SPEC and HZ;
-     if HZ != 1 also set LT.ticks.  */
+     if HZ != 1 also set TH.ticks.  */
   time_t time_spec;
   Lisp_Object hz;
-  struct lisp_time lt;
+  struct ticks_hz th;
   if (EQ (form, Qt))
     {
-      lt = lisp_time_struct (specified_time, CFORM_TICKS_HZ).lt;
-      struct timespec ts = ticks_hz_to_timespec (lt.ticks, lt.hz);
+      th = lisp_time_struct (specified_time, CFORM_TICKS_HZ).th;
+      struct timespec ts = ticks_hz_to_timespec (th.ticks, th.hz);
       if (! timespec_valid_p (ts))
 	time_overflow ();
       time_spec = ts.tv_sec;
-      hz = lt.hz;
+      hz = th.hz;
     }
   else
     {
@@ -1601,20 +1601,20 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
     sec = make_fixnum (local_tm.tm_sec);
   else
     {
-      /* Let TICKS = HZ * LOCAL_TM.tm_sec + mod (LT.ticks, HZ)
+      /* Let TICKS = HZ * LOCAL_TM.tm_sec + mod (TH.ticks, HZ)
 	 and SEC = (TICKS . HZ).  */
       Lisp_Object ticks;
       intmax_t n;
-      if (FASTER_TIMEFNS && FIXNUMP (lt.ticks) && FIXNUMP (hz)
+      if (FASTER_TIMEFNS && FIXNUMP (th.ticks) && FIXNUMP (hz)
 	  && !ckd_mul (&n, XFIXNUM (hz), local_tm.tm_sec)
-	  && !ckd_add (&n, n, (XFIXNUM (lt.ticks) % XFIXNUM (hz)
-			       + (XFIXNUM (lt.ticks) % XFIXNUM (hz) < 0
+	  && !ckd_add (&n, n, (XFIXNUM (th.ticks) % XFIXNUM (hz)
+			       + (XFIXNUM (th.ticks) % XFIXNUM (hz) < 0
 				  ? XFIXNUM (hz) : 0))))
 	ticks = make_int (n);
       else
 	{
 	  mpz_fdiv_r (mpz[0],
-		      *bignum_integer (&mpz[0], lt.ticks),
+		      *bignum_integer (&mpz[0], th.ticks),
 		      *bignum_integer (&mpz[1], hz));
 	  mpz_addmul_ui (mpz[0], *bignum_integer (&mpz[1], hz),
 			 local_tm.tm_sec);
@@ -1741,18 +1741,18 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0,
       yeararg = args[5];
     }
 
-  /* Let SEC = floor (LT.ticks / HZ), with SUBSECTICKS the remainder.  */
-  struct lisp_time lt = decode_lisp_time (secarg, CFORM_TICKS_HZ).time.lt;
-  Lisp_Object hz = lt.hz, sec, subsecticks;
+  /* Let SEC = floor (TH.ticks / HZ), with SUBSECTICKS the remainder.  */
+  struct ticks_hz th = decode_lisp_time (secarg, CFORM_TICKS_HZ).time.th;
+  Lisp_Object hz = th.hz, sec, subsecticks;
   if (FASTER_TIMEFNS && BASE_EQ (hz, make_fixnum (1)))
     {
-      sec = lt.ticks;
+      sec = th.ticks;
       subsecticks = make_fixnum (0);
     }
   else
     {
       mpz_fdiv_qr (mpz[0], mpz[1],
-		   *bignum_integer (&mpz[0], lt.ticks),
+		   *bignum_integer (&mpz[0], th.ticks),
 		   *bignum_integer (&mpz[1], hz));
       sec = make_integer_mpz ();
       mpz_swap (mpz[0], mpz[1]);
@@ -1780,8 +1780,8 @@ DEFUN ("encode-time", Fencode_time, Sencode_time, 1, MANY, 0,
 	    : INT_TO_INTEGER (value));
   else
     {
-      struct lisp_time val1 = { INT_TO_INTEGER (value), make_fixnum (1) };
-      Lisp_Object secticks = lisp_time_hz_ticks (val1, hz);
+      struct ticks_hz val1 = { INT_TO_INTEGER (value), make_fixnum (1) };
+      Lisp_Object secticks = ticks_hz_hz_ticks (val1, hz);
       Lisp_Object ticks = lispint_arith (secticks, subsecticks, false);
       return Fcons (ticks, hz);
     }
@@ -1812,19 +1812,19 @@ DEFUN ("time-convert", Ftime_convert, Stime_convert, 1, 2, 0,
   /* FIXME: Any reason why we don't offer a `float` output format option as
      well, since we accept it as input?  */
   struct form_time form_time = decode_lisp_time (time, CFORM_TICKS_HZ);
-  struct lisp_time t = form_time.time.lt;
+  struct ticks_hz t = form_time.time.th;
   form = (!NILP (form) ? maybe_remove_pos_from_symbol (form)
 	  : current_time_list ? Qlist : Qt);
   if (BASE_EQ (form, Qlist))
     return ticks_hz_list4 (t.ticks, t.hz);
   if (BASE_EQ (form, Qinteger))
-    return FASTER_TIMEFNS && INTEGERP (time) ? time : lisp_time_seconds (t);
+    return FASTER_TIMEFNS && INTEGERP (time) ? time : ticks_hz_seconds (t);
   if (BASE_EQ (form, Qt))
     form = t.hz;
   if (FASTER_TIMEFNS
       && form_time.form == TIMEFORM_TICKS_HZ && BASE_EQ (form, XCDR (time)))
     return time;
-  return Fcons (lisp_time_hz_ticks (t, form), form);
+  return Fcons (ticks_hz_hz_ticks (t, form), form);
 }
 
 DEFUN ("current-time", Fcurrent_time, Scurrent_time, 0, 0, 0,
-- 
2.34.1


[-- Attachment #9: 0008-Reduce-size-of-integer-product-in-timefns.patch --]
[-- Type: text/x-patch, Size: 2307 bytes --]

From 3c0cdb365ae2bb9a21948c081de69356263112eb Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Jul 2024 02:37:55 +0200
Subject: [PATCH 08/17] Reduce size of integer product in timefns
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/timefns.c (emacs_gcd): New static function.
(ticks_hz_hz_ticks): Use it to reduce the size of the integer
product in the common case when converting from ns to ps.  For
that, we need to multiply t.ticks only by 10³, not multiply by
10¹² and then divide by 10⁹.  This avoids the need to use bignums
in a significant number of cases.
---
 src/timefns.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 0c5f3bf3ff1..0a34bda28c7 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -748,6 +748,15 @@ timespec_ticks (struct timespec t)
   return make_integer_mpz ();
 }
 
+/* Return greatest common divisor of positive A and B.  */
+static EMACS_INT
+emacs_gcd (EMACS_INT a, EMACS_INT b)
+{
+  for (EMACS_INT r; (r = a % b) != 0; a = b, b = r)
+    continue;
+  return b;
+}
+
 /* Convert T to a Lisp integer counting HZ ticks, taking the floor.
    Assume T is valid, but check HZ.  */
 static Lisp_Object
@@ -766,11 +775,22 @@ ticks_hz_hz_ticks (struct ticks_hz t, Lisp_Object hz)
 	invalid_hz (hz);
 
       /* For speed, use intmax_t arithmetic if it will do.  */
-      intmax_t ticks;
-      if (FASTER_TIMEFNS && FIXNUMP (t.ticks) && FIXNUMP (t.hz)
-	  && !ckd_mul (&ticks, XFIXNUM (t.ticks), XFIXNUM (hz)))
-	return make_int (ticks / XFIXNUM (t.hz)
-			 - (ticks % XFIXNUM (t.hz) < 0));
+      if (FASTER_TIMEFNS && FIXNUMP (t.ticks) && FIXNUMP (t.hz))
+	{
+	  /* Reduce T.hz and HZ by their GCD, to avoid some intmax_t
+	     overflows that would occur in T.ticks * HZ.  */
+	  EMACS_INT ithz = XFIXNUM (t.hz), ihz = XFIXNUM (hz);
+	  EMACS_INT d = emacs_gcd (ithz, ihz);
+	  ithz /= d;
+	  ihz /= d;
+
+	  intmax_t ticks;
+	  if (!ckd_mul (&ticks, XFIXNUM (t.ticks), ihz))
+	    return make_int (ticks / ithz - (ticks % ithz < 0));
+
+	  t.hz = make_fixnum (ithz);
+	  hz = make_fixnum (ihz);
+	}
     }
   else if (! (BIGNUMP (hz) && 0 < mpz_sgn (*xbignum_val (hz))))
     invalid_hz (hz);
-- 
2.34.1


[-- Attachment #10: 0009-In-timefns-prefer-ui-mul-and-div.patch --]
[-- Type: text/x-patch, Size: 1473 bytes --]

From 4823ebd9ec3a5160f95aade0ec87d0218d1f19f3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Jul 2024 10:23:31 +0200
Subject: [PATCH 09/17] In timefns, prefer ui mul and div

* src/timefns.c (ticks_hz_hz_ticks): If the multiplier
is a fixnum that fits in unsigned long, use mpz_mul_ui
instead of the more-expensive mpz_mul.  Similarly, if the
divisor is a fixnum that fits in unsigned long, use
mpz_fdiv_q_ui instead of mpz_fdiv_q.
---
 src/timefns.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 0a34bda28c7..0df7d1f4363 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -796,10 +796,15 @@ ticks_hz_hz_ticks (struct ticks_hz t, Lisp_Object hz)
     invalid_hz (hz);
 
   /* Fall back on bignum arithmetic.  */
-  mpz_mul (mpz[0],
-	   *bignum_integer (&mpz[0], t.ticks),
-	   *bignum_integer (&mpz[1], hz));
-  mpz_fdiv_q (mpz[0], mpz[0], *bignum_integer (&mpz[1], t.hz));
+  mpz_t const *zticks = bignum_integer (&mpz[0], t.ticks);
+  if (FASTER_TIMEFNS && FIXNUMP (hz) && XFIXNUM (hz) <= ULONG_MAX)
+    mpz_mul_ui (mpz[0], *zticks, XFIXNUM (hz));
+  else
+    mpz_mul (mpz[0], *zticks, *bignum_integer (&mpz[1], hz));
+  if (FASTER_TIMEFNS && FIXNUMP (t.hz) && XFIXNUM (t.hz) <= ULONG_MAX)
+    mpz_fdiv_q_ui (mpz[0], mpz[0], XFIXNUM (t.hz));
+  else
+    mpz_fdiv_q (mpz[0], mpz[0], *bignum_integer (&mpz[1], t.hz));
   return make_integer_mpz ();
 }
 
-- 
2.34.1


[-- Attachment #11: 0010-In-timefns-do-gcd-reduction-more-often.patch --]
[-- Type: text/x-patch, Size: 1594 bytes --]

From 75dcc45c1589a5b42b8c24af448c1e7013bdf687 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Jul 2024 10:36:35 +0200
Subject: [PATCH 10/17] In timefns, do gcd reduction more often
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/timefns.c (ticks_hz_hz_ticks): Reduce by gcd
even if t.ticks is not a fixnum, since that’s easy.
---
 src/timefns.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 0df7d1f4363..ba1ba10a809 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -774,8 +774,8 @@ ticks_hz_hz_ticks (struct ticks_hz t, Lisp_Object hz)
       if (XFIXNUM (hz) <= 0)
 	invalid_hz (hz);
 
-      /* For speed, use intmax_t arithmetic if it will do.  */
-      if (FASTER_TIMEFNS && FIXNUMP (t.ticks) && FIXNUMP (t.hz))
+      /* Prefer non-bignum arithmetic to speed up common cases.  */
+      if (FASTER_TIMEFNS && FIXNUMP (t.hz))
 	{
 	  /* Reduce T.hz and HZ by their GCD, to avoid some intmax_t
 	     overflows that would occur in T.ticks * HZ.  */
@@ -784,9 +784,12 @@ ticks_hz_hz_ticks (struct ticks_hz t, Lisp_Object hz)
 	  ithz /= d;
 	  ihz /= d;
 
-	  intmax_t ticks;
-	  if (!ckd_mul (&ticks, XFIXNUM (t.ticks), ihz))
-	    return make_int (ticks / ithz - (ticks % ithz < 0));
+	  if (FIXNUMP (t.ticks))
+	    {
+	      intmax_t ticks;
+	      if (!ckd_mul (&ticks, XFIXNUM (t.ticks), ihz))
+		return make_int (ticks / ithz - (ticks % ithz < 0));
+	    }
 
 	  t.hz = make_fixnum (ithz);
 	  hz = make_fixnum (ihz);
-- 
2.34.1


[-- Attachment #12: 0011-In-timefns-call-natnump-only-for-non-fixnums.patch --]
[-- Type: text/x-patch, Size: 892 bytes --]

From 46978e54efa91fa4489a22c6b59de0d2b66f67e1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Jul 2024 11:04:18 +0200
Subject: [PATCH 11/17] In timefns, call natnump only for non-fixnums

* src/timefns.c (decode_time_components): Call Fnatnump only
for non-fixnums, as we need to special-case 0 anyway.
---
 src/timefns.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/timefns.c b/src/timefns.c
index ba1ba10a809..45a0930f8a8 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -895,7 +895,7 @@ decode_time_components (enum timeform form,
 
     case TIMEFORM_TICKS_HZ:
       if (! (INTEGERP (high)
-	     && !NILP (Fnatnump (low)) && !BASE_EQ (low, make_fixnum (0))))
+	     && (FIXNUMP (low) ? 0 < XFIXNUM (low) : !NILP (Fnatnump (low)))))
 	return (struct err_time) { .err = EINVAL };
       ticks = high;
       hz = low;
-- 
2.34.1


[-- Attachment #13: 0012-New-FASTER_BIGNUM-macro-to-test-slow-path-code.patch --]
[-- Type: text/x-patch, Size: 1690 bytes --]

From d566616f528a55795abeaf9ea48833a6f5488759 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Jul 2024 12:27:36 +0200
Subject: [PATCH 12/17] New FASTER_BIGNUM macro to test slow-path code

* src/bignum.h (FASTER_BIGNUM): New macro.
(mpz_set_intmax, mpz_set_uintmax): Optimize only if FASTER_BIGNUM.
Also, use ckd_add to test for overflow instead of doing it by hand.
---
 src/bignum.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/bignum.h b/src/bignum.h
index 2749f8370d0..54ba0cde410 100644
--- a/src/bignum.h
+++ b/src/bignum.h
@@ -25,6 +25,12 @@ #define BIGNUM_H
 #include <gmp.h>
 #include "lisp.h"
 
+/* Compile with -DFASTER_BIGNUM=0 to disable common optimizations and
+   allow easier testing of some slow-path code.  */
+#ifndef FASTER_BIGNUM
+# define FASTER_BIGNUM 1
+#endif
+
 /* Number of data bits in a limb.  */
 #ifndef GMP_NUMB_BITS
 enum { GMP_NUMB_BITS = TYPE_WIDTH (mp_limb_t) };
@@ -68,16 +74,18 @@ mpz_set_intmax (mpz_t result, intmax_t v)
   /* mpz_set_si works in terms of long, but Emacs may use a wider
      integer type, and so sometimes will have to construct the mpz_t
      by hand.  */
-  if (LONG_MIN <= v && v <= LONG_MAX)
-    mpz_set_si (result, v);
+  long int i;
+  if (FASTER_BIGNUM && !ckd_add (&i, v, 0))
+    mpz_set_si (result, i);
   else
     mpz_set_intmax_slow (result, v);
 }
 INLINE void ARG_NONNULL ((1))
 mpz_set_uintmax (mpz_t result, uintmax_t v)
 {
-  if (v <= ULONG_MAX)
-    mpz_set_ui (result, v);
+  unsigned long int i;
+  if (FASTER_BIGNUM && !ckd_add (&i, v, 0))
+    mpz_set_ui (result, i);
   else
     mpz_set_uintmax_slow (result, v);
 }
-- 
2.34.1


[-- Attachment #14: 0013-Optimize-smallish-mpz-to-native-int-conversion.patch --]
[-- Type: text/x-patch, Size: 2052 bytes --]

From 6923145c10b50d86d133c7f7a5bb3c407ae246e5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Jul 2024 12:42:57 +0200
Subject: [PATCH 13/17] Optimize smallish mpz to native int conversion

* src/bignum.c (make_integer_mpz, mpz_to_intmax):
If FASTER_BIGNUM, optimize the common case where the value fits in
long int.  In this case we can use mpz_fits_slong_p and mpz_get_si
instead of looping with mpz_getlimbn.
(mpz_to_uintmax): Likewise for unsigned long int and mpz_get_ui.
---
 src/bignum.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/bignum.c b/src/bignum.c
index 1fe195d78ea..7589691dd0c 100644
--- a/src/bignum.c
+++ b/src/bignum.c
@@ -145,9 +145,19 @@ make_neg_biguint (uintmax_t n)
 Lisp_Object
 make_integer_mpz (void)
 {
+  if (FASTER_BIGNUM && mpz_fits_slong_p (mpz[0]))
+    {
+      long int v = mpz_get_si (mpz[0]);
+      if (!FIXNUM_OVERFLOW_P (v))
+	return make_fixnum (v);
+    }
+
   size_t bits = mpz_sizeinbase (mpz[0], 2);
 
-  if (bits <= FIXNUM_BITS)
+  if (! (FASTER_BIGNUM
+	 && FIXNUM_OVERFLOW_P (LONG_MIN)
+	 && FIXNUM_OVERFLOW_P (LONG_MAX))
+      && bits <= FIXNUM_BITS)
     {
       EMACS_INT v = 0;
       int i = 0, shift = 0;
@@ -216,6 +226,17 @@ mpz_set_uintmax_slow (mpz_t result, uintmax_t v)
 bool
 mpz_to_intmax (mpz_t const z, intmax_t *pi)
 {
+  if (FASTER_BIGNUM)
+    {
+      if (mpz_fits_slong_p (z))
+	{
+	  *pi = mpz_get_si (z);
+	  return true;
+	}
+      if (LONG_MIN <= INTMAX_MIN && INTMAX_MAX <= LONG_MAX)
+	return false;
+    }
+
   ptrdiff_t bits = mpz_sizeinbase (z, 2);
   bool negative = mpz_sgn (z) < 0;
 
@@ -246,6 +267,17 @@ mpz_to_intmax (mpz_t const z, intmax_t *pi)
 bool
 mpz_to_uintmax (mpz_t const z, uintmax_t *pi)
 {
+  if (FASTER_BIGNUM)
+    {
+      if (mpz_fits_ulong_p (z))
+	{
+	  *pi = mpz_get_ui (z);
+	  return true;
+	}
+      if (UINTMAX_MAX <= ULONG_MAX)
+	return false;
+    }
+
   if (mpz_sgn (z) < 0)
     return false;
   ptrdiff_t bits = mpz_sizeinbase (z, 2);
-- 
2.34.1


[-- Attachment #15: 0014-In-timefns.c-avoid-by-hand-overflow-checking.patch --]
[-- Type: text/x-patch, Size: 5121 bytes --]

From 39d9df2181209d700f41b2064a02e5cea42c1564 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Jul 2024 12:57:01 +0200
Subject: [PATCH 14/17] In timefns.c avoid by-hand overflow checking

Prefer functions like ckd_add to do overflow checking, instead of
doing it by hand, to simplify and I hope to make things a bit less
error prone.
* src/timefns.c (TIME_T_MIN, TIME_T_MAX): Remove.  All by-hand
overflow checking replaced with calls to ckd_add or ckd_mul.
(s_ns_to_timespec): New static function, that uses ckd_add
instead of by-hand overflow checking.
(ticks_hz_to_timespec): Use it.
(check_tm_member): Use mpz_fits_sint_p and mpz_get_si rather
than mpz_to_intmax and by-hand overflow checking.
---
 src/timefns.c | 76 ++++++++++++++++++---------------------------------
 1 file changed, 27 insertions(+), 49 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 45a0930f8a8..6949c83dbcb 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -60,13 +60,6 @@ Copyright (C) 1985-1987, 1989, 1993-2024 Free Software Foundation, Inc.
 # define HAVE_TM_GMTOFF false
 #endif
 
-#ifndef TIME_T_MIN
-# define TIME_T_MIN TYPE_MINIMUM (time_t)
-#endif
-#ifndef TIME_T_MAX
-# define TIME_T_MAX TYPE_MAXIMUM (time_t)
-#endif
-
 /* Compile with -DFASTER_TIMEFNS=0 to disable common optimizations and
    allow easier testing of some slow-path code.  */
 #ifndef FASTER_TIMEFNS
@@ -127,10 +120,14 @@ make_timeval (struct timespec t)
     {
       if (tv.tv_usec < 999999)
 	tv.tv_usec++;
-      else if (tv.tv_sec < TIME_T_MAX)
+      else
 	{
-	  tv.tv_sec++;
-	  tv.tv_usec = 0;
+	  time_t s1;
+	  if (!ckd_add (&s1, tv.tv_sec, 1))
+	    {
+	      tv.tv_sec = s1;
+	      tv.tv_usec = 0;
+	    }
 	}
     }
 
@@ -492,18 +489,23 @@ mpz_time (mpz_t const z, time_t *t)
   if (TYPE_SIGNED (time_t))
     {
       intmax_t i;
-      if (! (mpz_to_intmax (z, &i) && TIME_T_MIN <= i && i <= TIME_T_MAX))
-	return false;
-      *t = i;
+      return mpz_to_intmax (z, &i) && !ckd_add (t, i, 0);
     }
   else
     {
       uintmax_t i;
-      if (! (mpz_to_uintmax (z, &i) && i <= TIME_T_MAX))
-	return false;
-      *t = i;
+      return mpz_to_uintmax (z, &i) && !ckd_add (t, i, 0);
     }
-  return true;
+}
+
+/* Return a valid timespec (S, N) if S is in time_t range,
+   an invalid timespec otherwise.  */
+static struct timespec
+s_ns_to_timespec (intmax_t s, long int ns)
+{
+  time_t sec;
+  long int nsec = ckd_add (&sec, s, 0) ? -1 : ns;
+  return make_timespec (sec, nsec);
 }
 
 /* Components of a Lisp timestamp (TICKS . HZ).  Using this C struct can
@@ -522,7 +524,6 @@ mpz_time (mpz_t const z, time_t *t)
 static struct timespec
 ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
 {
-  struct timespec result = invalid_timespec ();
   int ns;
   mpz_t *q = &mpz[0];
   mpz_t const *qt = q;
@@ -539,33 +540,16 @@ ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
 	  ns = XFIXNUM (ticks) % TIMESPEC_HZ;
 	  if (ns < 0)
 	    s--, ns += TIMESPEC_HZ;
-	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
-	      && s <= TIME_T_MAX)
-	    {
-	      result.tv_sec = s;
-	      result.tv_nsec = ns;
-	    }
-	  return result;
+	  return s_ns_to_timespec (s, ns);
 	}
-      else
-	ns = mpz_fdiv_q_ui (*q, *xbignum_val (ticks), TIMESPEC_HZ);
+      ns = mpz_fdiv_q_ui (*q, *xbignum_val (ticks), TIMESPEC_HZ);
     }
   else if (FASTER_TIMEFNS && BASE_EQ (hz, make_fixnum (1)))
     {
       ns = 0;
       if (FIXNUMP (ticks))
-	{
-	  EMACS_INT s = XFIXNUM (ticks);
-	  if ((TYPE_SIGNED (time_t) ? TIME_T_MIN <= s : 0 <= s)
-	      && s <= TIME_T_MAX)
-	    {
-	      result.tv_sec = s;
-	      result.tv_nsec = ns;
-	    }
-	  return result;
-	}
-      else
-	qt = xbignum_val (ticks);
+	return s_ns_to_timespec (XFIXNUM (ticks), ns);
+      qt = xbignum_val (ticks);
     }
   else
     {
@@ -577,12 +561,7 @@ ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
   /* Check that Q fits in time_t, not merely in RESULT.tv_sec.  With some MinGW
      versions, tv_sec is a 64-bit type, whereas time_t is a 32-bit type.  */
   time_t sec;
-  if (mpz_time (*qt, &sec))
-    {
-      result.tv_sec = sec;
-      result.tv_nsec = ns;
-    }
-  return result;
+  return mpz_time (*qt, &sec) ? make_timespec (sec, ns) : invalid_timespec ();
 }
 
 /* C timestamp forms.  This enum is passed to conversion functions to
@@ -700,7 +679,7 @@ ticks_hz_list4 (Lisp_Object ticks, Lisp_Object hz)
   int us = mpz_get_ui (mpz[1]);
 #endif
 
-  /* mpz[0] = floor (mpz[0] / 1 << LO_TIME_BITS), with lo = remainder.  */
+  /* mpz[0] = floor (mpz[0] / (1 << LO_TIME_BITS)), with LO = remainder.  */
   unsigned long ulo = mpz_get_ui (mpz[0]);
   if (mpz_sgn (mpz[0]) < 0)
     ulo = -ulo;
@@ -1686,10 +1665,9 @@ check_tm_member (Lisp_Object obj, int offset)
     {
       CHECK_INTEGER (obj);
       mpz_sub_ui (mpz[0], *bignum_integer (&mpz[0], obj), offset);
-      intmax_t i;
-      if (! (mpz_to_intmax (mpz[0], &i) && INT_MIN <= i && i <= INT_MAX))
+      if (!mpz_fits_sint_p (mpz[0]))
 	time_overflow ();
-      return i;
+      return mpz_get_si (mpz[0]);
     }
 }
 
-- 
2.34.1


[-- Attachment #16: 0015-Decode-current-time-directly-to-timespec.patch --]
[-- Type: text/x-patch, Size: 1201 bytes --]

From 5a982c3e7b1e8931530e110bb5388047829fa4fe Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Jul 2024 15:03:50 +0200
Subject: [PATCH 15/17] Decode current time directly to timespec

* src/timefns.c (decode_time_components): If FASTER_TIMEFNS, when
returning the current time and the desired form is struct timespec
or time_t, return it directly rather than converting it to struct
ticks_hz and then to struct timespec.  This can avoid some mpz
calculations and/or bignums.
---
 src/timefns.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 6949c83dbcb..ad39d1307cd 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -884,8 +884,14 @@ decode_time_components (enum timeform form,
       eassume (false);
 
     case TIMEFORM_NIL:
-      ticks = timespec_ticks (current_timespec ());
-      hz = timespec_hz;
+      {
+	struct timespec now = current_timespec ();
+	if (FASTER_TIMEFNS
+	    && (cform == CFORM_TIMESPEC || cform == CFORM_SECS_ONLY))
+	  return (struct err_time) { .time = { .ts = now } };
+	ticks = timespec_ticks (now);
+	hz = timespec_hz;
+      }
       break;
 
     default:
-- 
2.34.1


[-- Attachment #17: 0016-Avoid-mpz-for-some-common-timestamp-cases.patch --]
[-- Type: text/x-patch, Size: 5038 bytes --]

From 7291b20806addd95770a4b58da4049d7a14ea871 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Jul 2024 15:28:58 +0200
Subject: [PATCH 16/17] Avoid mpz for some common timestamp cases
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Performance problem reported by Gerd Möllmann and Mattias Engdegård in:
https://lists.gnu.org/r/emacs-devel/2024-06/msg00530.html
https://lists.gnu.org/r/emacs-devel/2024-06/msg00539.html
* src/timefns.c (CFORM_SECS_ONLY): The exact tv_nsec value is now
ignored if nonnegative (i.e., the only thing that matters is that
it’s nonnegative).
(decode_time_components): Use intmax_t instead of mpz arithmetic
if the tick count fits.  Add another ‘default: eassume (false);’
so that the revised code pacifies --enable-gcc-warnings with GCC
11.4.0 on x86-64.
---
 src/timefns.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 73 insertions(+), 11 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index ad39d1307cd..8c30016360d 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -570,7 +570,8 @@ ticks_hz_to_timespec (Lisp_Object ticks, Lisp_Object hz)
   {
     CFORM_TICKS_HZ, /* struct ticks_hz */
     CFORM_TIMESPEC, /* struct timespec */
-    CFORM_SECS_ONLY, /* struct timespec but tv_nsec == 0 if timespec valid */
+    CFORM_SECS_ONLY, /* struct timespec but tv_nsec irrelevant
+			if timespec valid */
     CFORM_DOUBLE /* double */
   };
 
@@ -894,11 +895,22 @@ decode_time_components (enum timeform form,
       }
       break;
 
-    default:
-      if (! (INTEGERP (high) && INTEGERP (low)
-	     && FIXNUMP (usec) && FIXNUMP (psec)))
-	return (struct err_time) { .err = EINVAL };
+    case TIMEFORM_HI_LO:
+      hz = make_fixnum (1);
+      goto check_high_low;
+
+    case TIMEFORM_HI_LO_US:
+      hz = make_fixnum (1000000);
+      goto check_high_low_usec;
 
+    case TIMEFORM_HI_LO_US_PS:
+      hz = trillion;
+      if (!FIXNUMP (psec))
+	return (struct err_time) { .err = EINVAL };
+    check_high_low_usec:
+      if (!FIXNUMP (usec))
+	return (struct err_time) { .err = EINVAL };
+    check_high_low:
       {
 	EMACS_INT us = XFIXNUM (usec);
 	EMACS_INT ps = XFIXNUM (psec);
@@ -906,25 +918,73 @@ decode_time_components (enum timeform form,
 	/* Normalize out-of-range lower-order components by carrying
 	   each overflow into the next higher-order component.  */
 	us += ps / 1000000 - (ps % 1000000 < 0);
+	EMACS_INT s_from_us_ps = us / 1000000 - (us % 1000000 < 0);
+	ps = ps % 1000000 + 1000000 * (ps % 1000000 < 0);
+	us = us % 1000000 + 1000000 * (us % 1000000 < 0);
+
+	if (FASTER_TIMEFNS && FIXNUMP (high) && FIXNUMP (low))
+	  {
+	    /* Use intmax_t arithmetic if the tick count fits.  */
+	    intmax_t iticks;
+	    bool v = false;
+	    v |= ckd_mul (&iticks, XFIXNUM (high), 1 << LO_TIME_BITS);
+	    v |= ckd_add (&iticks, iticks, XFIXNUM (low) + s_from_us_ps);
+	    if (!v)
+	      {
+		if (cform == CFORM_TIMESPEC || cform == CFORM_SECS_ONLY)
+		  return (struct err_time) {
+		    .time = {
+		      .ts = s_ns_to_timespec (iticks, us * 1000 + ps / 1000)
+		    }
+		  };
+
+		switch (form)
+		  {
+		  case TIMEFORM_HI_LO:
+		    break;
+
+		  case TIMEFORM_HI_LO_US:
+		    v |= ckd_mul (&iticks, iticks, 1000000);
+		    v |= ckd_add (&iticks, iticks, us);
+		    break;
+
+		  case TIMEFORM_HI_LO_US_PS:
+		    {
+		      int_fast64_t million = 1000000;
+		      v |= ckd_mul (&iticks, iticks, TRILLION);
+		      v |= ckd_add (&iticks, iticks, us * million + ps);
+		    }
+		    break;
+
+		  default:
+		    eassume (false);
+		  }
+
+		if (!v)
+		  return (struct err_time) {
+		    .time = decode_ticks_hz (make_int (iticks), hz, cform)
+		  };
+	      }
+	  }
+
+	if (! (INTEGERP (high) && INTEGERP (low)))
+	  return (struct err_time) { .err = EINVAL };
+
 	mpz_t *s = &mpz[1];
-	mpz_set_intmax (*s, us / 1000000 - (us % 1000000 < 0));
+	mpz_set_intmax (*s, s_from_us_ps);
 	mpz_add (*s, *s, *bignum_integer (&mpz[0], low));
 	mpz_addmul_ui (*s, *bignum_integer (&mpz[0], high), 1 << LO_TIME_BITS);
-	ps = ps % 1000000 + 1000000 * (ps % 1000000 < 0);
-	us = us % 1000000 + 1000000 * (us % 1000000 < 0);
 
 	switch (form)
 	  {
 	  case TIMEFORM_HI_LO:
 	    /* Floats and nil were handled above, so it was an integer.  */
 	    mpz_swap (mpz[0], *s);
-	    hz = make_fixnum (1);
 	    break;
 
 	  case TIMEFORM_HI_LO_US:
 	    mpz_set_ui (mpz[0], us);
 	    mpz_addmul_ui (mpz[0], *s, 1000000);
-	    hz = make_fixnum (1000000);
 	    break;
 
 	  case TIMEFORM_HI_LO_US_PS:
@@ -938,7 +998,6 @@ decode_time_components (enum timeform form,
 		mpz_set_intmax (mpz[0], i * 1000000 + ps);
 		mpz_addmul (mpz[0], *s, ztrillion);
 	      #endif
-	      hz = trillion;
 	    }
 	    break;
 
@@ -948,6 +1007,9 @@ decode_time_components (enum timeform form,
 	ticks = make_integer_mpz ();
       }
       break;
+
+    default:
+      eassume (false);
     }
 
   return (struct err_time) { .time = decode_ticks_hz (ticks, hz, cform) };
-- 
2.34.1


[-- Attachment #18: 0017-Rename-timefns-static-function-lisp_time_struct.patch --]
[-- Type: text/x-patch, Size: 2792 bytes --]

From 7fe0564befaeb45f8f9f2f5753f2b24e1f891d15 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Jul 2024 15:40:47 +0200
Subject: [PATCH 17/17] Rename timefns static function lisp_time_struct

* src/timefns.c (lisp_time_cform): Rename from lisp_time_struct,
since it no longer returns a struct, and now accepts CFORM.
All uses changed.
---
 src/timefns.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/timefns.c b/src/timefns.c
index 8c30016360d..1e551009df8 100644
--- a/src/timefns.c
+++ b/src/timefns.c
@@ -1125,24 +1125,24 @@ list4_to_timespec (Lisp_Object high, Lisp_Object low,
   return err_time.err ? invalid_timespec () : err_time.time.ts;
 }
 
-/* Decode a Lisp list SPECIFIED_TIME that represents a time.
+/* Decode a Lisp time value SPECIFIED_TIME that represents a time.
    If SPECIFIED_TIME is nil, use the current time.
    Decode to CFORM form.
    Signal an error if SPECIFIED_TIME does not represent a time.  */
 static union c_time
-lisp_time_struct (Lisp_Object specified_time, enum cform cform)
+lisp_time_cform (Lisp_Object specified_time, enum cform cform)
 {
   return decode_lisp_time (specified_time, cform).time;
 }
 
-/* Decode a Lisp list SPECIFIED_TIME that represents a time.
+/* Decode a Lisp time value SPECIFIED_TIME that represents a time.
    Discard any low-order (sub-ns) resolution.
    If SPECIFIED_TIME is nil, use the current time.
    Signal an error if SPECIFIED_TIME does not represent a timespec.  */
 struct timespec
 lisp_time_argument (Lisp_Object specified_time)
 {
-  struct timespec t = lisp_time_struct (specified_time, CFORM_TIMESPEC).ts;
+  struct timespec t = lisp_time_cform (specified_time, CFORM_TIMESPEC).ts;
   if (! timespec_valid_p (t))
     time_overflow ();
   return t;
@@ -1346,8 +1346,8 @@ time_cmp (Lisp_Object a, Lisp_Object b)
 
   /* Compare (ATICKS . AZ) to (BTICKS . BHZ) by comparing
      ATICKS * BHZ to BTICKS * AHZ.  */
-  struct ticks_hz ta = lisp_time_struct (a, CFORM_TICKS_HZ).th;
-  struct ticks_hz tb = lisp_time_struct (b, CFORM_TICKS_HZ).th;
+  struct ticks_hz ta = lisp_time_cform (a, CFORM_TICKS_HZ).th;
+  struct ticks_hz tb = lisp_time_cform (b, CFORM_TICKS_HZ).th;
   mpz_t const *za = bignum_integer (&mpz[0], ta.ticks);
   mpz_t const *zb = bignum_integer (&mpz[1], tb.ticks);
   if (! (FASTER_TIMEFNS && BASE_EQ (ta.hz, tb.hz)))
@@ -1630,7 +1630,7 @@ DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 3, 0,
   struct ticks_hz th;
   if (EQ (form, Qt))
     {
-      th = lisp_time_struct (specified_time, CFORM_TICKS_HZ).th;
+      th = lisp_time_cform (specified_time, CFORM_TICKS_HZ).th;
       struct timespec ts = ticks_hz_to_timespec (th.ticks, th.hz);
       if (! timespec_valid_p (ts))
 	time_overflow ();
-- 
2.34.1

Re: Question about bignum usage

[Mattias Engdegård]

11 juli 2024 kl. 16.10 skrev Paul Eggert <eggert@cs.ucla.edu>:

> I installed the attached. 0017 should should address the performance problem with decode_timer. The other patches refactor and address some related issues.

That was a lot more ambitious than I had in mind, thank you!

I pushed a tiny simplification. More can be done. For example, time decoding output sometimes depends on the exact form of the input:

(time-convert '(0 3) t) => (3 . 1)
(time-convert '(0 3 0) t) => (3000000 . 1000000)
(time-convert '(0 3 0 0) t) => (3000000000000 . 1000000000000)

Does it make sense to distinguish (A B), (A B 0) and (A B 0 0) like this? Some kind of performance hack to keep denominators the same?

> As I understand it decode_timer was the locus of the performance issue. If other places are also involved, please let me know.

It probably dealt with the most noticeable parts but the timer system doesn't appear to be short on slow code.
For example, does timer_check really need to duplicate the time queues each time they are checked for an event to run, even if there isn't one?
Not to mention the baroque time representation of `timer` objects and the cost of time-less-p and (especially) timer--time-less-p.

Can we make changes to the `timer` struct now? Apparently there were some holdouts in 2012 (which is why the `psec` slot is where it is) but there has to be a limit to how we have to support old badly written code.

Re: Question about bignum usage

[Paul Eggert]

On 2024-07-13 02:43, Mattias Engdegård wrote:
>   time decoding output sometimes depends on the exact form of the input:
> 
> (time-convert '(0 3) t) => (3 . 1)
> (time-convert '(0 3 0) t) => (3000000 . 1000000)
> (time-convert '(0 3 0 0) t) => (3000000000000 . 1000000000000)
> 
> Does it make sense to distinguish (A B), (A B 0) and (A B 0 0) like
> this?

Yes, because the input form has the same effective precision as the 
output form. One would not want (time-convert '(0 3 0 0) t) and
(time-convert '(0 3 0 1) t) to generate timestamps with different 
precisions.

If one knows the input timestamp has a particular resolution regardless 
of its format, μs for example, one can give that resolution to 
time-convert, e.g., (time-convert TIMESTAMP 1000000).