weavejester / eftest

Fast and pretty Clojure test runner
425 stars 40 forks source link

Prevent starvation in parallel test execution #30

Closed ztellman closed 7 years ago

ztellman commented 7 years ago

The current parallel implementation uses pmap, which allows a single long-lived test to prevent subsequent tests from being executed. Consider this example:

(->> 1e3 
  range 
  (map (fn [n] #(println n))) 
  (cons #(Thread/sleep 1e4)) 
  (pmap #(%)))

This will execute the first "block" of tasks immediately (the exact size of the block depending your number of cores), and then sleep for 10 seconds before executing the rest. Conversely, if we do this with the run-in-parallel function I've added:

(->> 1e3 
  range 
  (map (fn [n] #(println n))) 
  (cons #(Thread/sleep 1e4)) 
  run-in-parallel)

Then it will execute all thousand tasks immediately, and then wait 10 seconds for the first task to finally complete before returning. This is a very non-theoretical improvement for me, as I'm running multi-hour test.check suites in parallel, and often seeing only a single core in use.

danielcompton commented 7 years ago

Would it be good to make this threadpool size configurable? If tests need to do some I/O then I think the total test time would be longer as the threadpool is sized to the number of CPUs?

ztellman commented 7 years ago

That's easy to add, if @weavejester likes the idea. I can also make the thread pool twice the number of cores (which is what pmap does) to handle that situation a bit better by default.

EDIT: just realized that pmap ADDS two to the number of cores, it's Netty which multiplies it by two. I still think the latter would be fine, fwiw.

weavejester commented 7 years ago

Making the thread pool size configurable seems reasonable. I think the default should be n+2, where n is the number of cores, to make it the same as pmap.

Could you also capitalize the first letter of your commit message, and align the arguments of ->>?

(I've written up a short guideline for contributing if you want more information, but I haven't worked out the best way of distributing it to my repositories. I'm leaning to a link in the README.)

ztellman commented 7 years ago

I'll punt on making it configurable (you know best how you'd like to expose those options), but I've updated the commit message, indentation, and default thread count.

weavejester commented 7 years ago

Thanks!