Speed-optimize '--each-while'.

[doublep]

I want to optimize several functions in the library. For a start, here is the first optimization. Tell me if it's OK to continue or if I shouldn't waste my time.

Reasoning: dash is low-level library used in many other places. It contains very generic functions that a purpose-agnostic and may or may not be used in performance-critical places. Implementation of the functions is also pretty simple, usually 1--10 lines. Therefore, I think, performance here is more important than code clarity.

This patch optimizes '--each-while':

• don't bind 'it' in a loop, it's slow: move it to encompassing 'let' instead;
• get rid of 'continue': we can just set 'list' to nil to break the loop early;
• rearrange loop body a bit so that there are fewer gotos in compiled code.

Benchmark:

(defvar --large-list-- (--map-indexed it-index (-repeat 1000 nil)))
(benchmark-run-compiled 10000
  (--each-while --large-list-- (< it 500)))
(benchmark-run-compiled 10000
  (--each-while-optimized --large-list-- (< it 500)))

Before: 0.33+ seconds here, after: 0.28+ s here (runs differ a lot, I just repeat them several times and pick the best.

[Fuco1]

I have no problems with this.

[magnars]

Hi! This is a great initiative. One issue: Do you have your FSF paperwork signed?

[doublep]

Yes, I signed them many years ago. You can find a few my changes in Emacs' ChangeLog and in AUTHORS.

[Fuco1]

Speaking about benchmarks, if we're going to be doing such an optimization project, it would make sense to prepare some suite we could run automatically, ideally covering all the functions.

Obviously you don't need to write a benchmark for every function, but we could at least discuss and come up with some framework, ideally as automatic as possible (something alike to how tests work now)

[doublep]

The problem is that benchmarking seems to give really wide result distribution. So, to get a good estimate you'd need to run each benchmark at least 10 times, if not 100.

[Fuco1]

I guess it has lot to do with GC and other internals too... there might be some ways to prepare "good states", or alternatively always run in a clean emacs instance (loading up one with -nw -q doesn't take long). Plus, if the whole benchmark runs less than 5 minutes I think it is acceptable (if we provide some way to run single benchmarks separately).

Though this is maybe not a task for dash but for some generic framework to be developed (a la ert). Maybe there even already is something like that, dunno.

[doublep]

Here is a simple try. I explicitly run GC before every benchmark-run-compiled. 10 is meant to be configurable later.

(defmacro --benchmark (&optional repetitions &rest forms)
  `(-let ((all-results nil))
     (--dotimes 10
       (garbage-collect)
       (!cons (benchmark-run-compiled ,repetitions ,@forms) all-results))
     (--benchmark--result 10 all-results)))

(defun --benchmark--result (num-tries all-results)
  (-let ((best-result (--min-by (- (nth 0 it) (nth 2 it)) all-results)))
    (if (= (nth 1 best-result) 0)
        (format "Best of %d tries: %.3f s" num-tries (nth 0 best-result))
      (format "Best of %d tries: %.3f s (%d GC runs)"
              num-tries (- (nth 0 best-result) (nth 2 best-result)) (nth 1 best-result)))))

(defvar --large-list-- (--map-indexed it-index (-repeat 1000 nil)))
(--benchmark 10000
  (--each-while --large-list-- (< it 500)))
(--benchmark 10000
  (--each-while-optimized --large-list-- (< it 500)))

[Fuco1]

Hm, does this let binding work? &as should be in the beginning, foo &as <destruct-here>.

I guess we could theoretically make it work both ways... but it is different from clojure at this point.

[doublep]

You are right. It "sort of works" only because I never use the benchmark variable, but it is wrong. I will commit a fix.

[doublep]

I squashed benchmark framework commits, also with a fix for that wrong &as you noticed.

[Fuco1]

This PR has got quite big. Would you be so kind and split it into two (ideally one commit per function), one for the optimization thing and another for the benchmarks?

I'm very much looking forward to getting this merged... we've neglected the PRs here for quite some time :)

[basil-conto]

@doublep Are you still interested in rebasing this work on top of latest master?

[basil-conto]

@doublep Are you still interested in rebasing this work on top of latest master?

Ping.