Unstarted cancelled futures should not run

[mainej]

The body of a promise created by p/future is always run, even if the promise is cancelled with p/cancel!. This keeps the common thread pool busier than it needs to be.

I'm not sure whether this is a bug in p/future, or if there's another promesa.core function that I should be using.

To be more concrete, let's look at what happens when you start more futures than can be processed simultaneously in the common thread pool, cancel all of them, then check which ones were actually run.

If you use p/future, all of the tasks are run. That is, the following will always return [1, 2, ... max-n].

(defn check [make-future]
  (let [results (atom [])
        common-parallelism (java.util.concurrent.ForkJoinPool/getCommonPoolParallelism)
        max-n (* 2 common-parallelism)
        process-result (fn [n]
                         (Thread/sleep (* 100 n))
                         (swap! results conj n))
        futs (doall
               (map (fn [n]
                      (make-future #(process-result n)))
                    (range 1 (inc max-n))))]
    ;; give executor time to start first `common-parallelism` tasks
    (Thread/sleep 20)
    ;; cancel all futures, before some have started, and before any are finished
    (run! p/cancel! futs)
    ;; wait until all futures _would_ have finished, if they had been run
    (Thread/sleep (* 200 max-n))
    ;; check which futures actually ran
    @results))

(check (fn [f] (p/future (f))))

This is in contrast to CompletableFuture/supplyAsync + Supplier/get, which never runs more tasks than are initially submitted to the pool. That is, the following always returns a vector containing fewer results, usually [1, 2, ... common-parallelism], but never more than common-parallelism of the ns.

(check (fn [f]
         (java.util.concurrent.CompletableFuture/supplyAsync
           (reify java.util.function.Supplier
             (get [_] (f))))))

[niwinz]

Hi @mainej I will look on it ASAP, I'm just right now over-saturated with other stuff and can't give time for this. Looks interesting

[niwinz]

You are right and there are a bug in future impl. I have fixed it locally and it will be released in next version

[mainej]

OK, great! Thanks for digging into this @niwinz ... I'm looking forward to the next release.

[niwinz]

should be fixed in the 9.0.462

[mainej]

Thanks @niwinz! I tested 9.0.462 and futures are cancelled as expected. See https://github.com/clojure-lsp/clojure-lsp/pull/1293