bug#81581: [PATCH 1/1] Add a LESSP argument to 'seq-min' and 'seq-max'

Eli Zaretskii <[email protected]> writes:

>> From: Philip Kaludercic <[email protected]>
>> Date: Sat, 08 Aug 2026 19:59:57 +0000
>> 
>> -(cl-defgeneric seq-min (sequence)
>> +(cl-defgeneric seq-min (sequence &optional lessp)
>>    "Return the smallest element of SEQUENCE.
>> -SEQUENCE must be a sequence of numbers or markers."
>> -  (apply #'min (seq-into sequence 'list)))
>> +Values are compared according to the optional parameter LESSP, which
>> +defaults to `value<'."
>> +  (unless lessp (setq lessp #'value<))
>> +  (let ((fresh (eval-when-compile (make-symbol "fresh"))))
>> +    (seq-reduce
>> +     (lambda (acc elt)
>> +       (cond
>> +        ((eq acc fresh) elt)
>> +        ((funcall lessp acc elt) acc)
>> +        (t elt)))
>> +     sequence
>> +     fresh)))
>
> How about optimizing for the default nil value of LESSP?  The original
> implementation should be faster than the modified one, so how about
> keeping the original performance for those who don't need a fancy
> comparison function?

To know how much this matters or not, here are some benchmarks;

  (let ((list (make-list 10000 nil)))
    (dotimes (i (length list))
      (setf (nth i list) (random)))
    (benchmark-run 100
      (seq-min list)))

  ;; old implementation
  (0.007135554 0 0.0)

  ;; new implementation
  (0.957282933 8 0.7767616650000093)

even if the sequence type is not a list, there is a difference, though
less pronounced:

  (let ((vec (make-vector 10000 nil)))
    (dotimes (i (length vec))
      (setf (aref vec i) (random)))
    (benchmark-run 100
      (seq-min-old vec)))

  ;; old
  (0.108933372 1 0.09555754100000513)

  ;; new
  (0.891958548 7 0.7179179070000146)

Of course, the performance difference ultimately boils down to the fact
that `min' (and `max') is built-in, but `min' only operates on numbers
and markers.  Unless we implement a `min' and `max' that operate on
`value<' in C -- which I think would also be useful -- then to optimize
we would have to also change the fallback of LESSP to < and retain the
restriction that SEQUENCE has to consist of numerical values.

My counter-argument is that seq has never been performance oriented, as
it uses dynamic dispatch and is non-destructive most of the time.  There
is no advantage of using `seq-min' compared to `min' if you are dealing
with lists.  With other sequences you can avoid having to call
`seq-into' by hand.  So I would advocate to have seq be a more
interesting function instead of just a convenience wrapper, even if that
comes at the cost of a slowdown.

Or did I misunderstand you, and you are advocating for a case
distinction on LESSP, and dispatch to `min' and `max' if LESSP is `<`
(instead of nil)?