@cgrand Looks good to me. The only thing that’s a bit of a problem is that moving the default executor will break cider-nrepl, as there was a bit of code using this implementation detail directly, but that’s not a big deal.

I guess it would also be nice to document the new behavior accordingly, although that’d be mostly for us - I highly doubt that anyone relied on the implementation details of the execution.

One more somewhat related thing is the handling of output that we discussed could be improved if we have one thread per session, but I guess this can be done later.

@dchelimsky The new Lein 2.8.2 is out! 🙂

@bozhidar another breaking change is that the configurable excutor to interruptible-eval is ignored — is it used in practice?

and what do you do in cider-nreplwith the executor?

Oh, that’s only for tests?

Having worked on thread management, I’m now wondering if I missed a documentation on the execution model (not the implementation). My understanding: • msgs from different session are concurrent • :eval msgs for a given session are serialized • non-`:eval` msgs for a given session are... concurrent?

> @bozhidar another breaking change is that the configurable excutor to interruptible-eval is ignored — is it used in practice?

It’s not.

> and what do you do in cider-nreplwith the executor?

It’s used directly in the test-runner - https://github.com/clojure-emacs/cider-nrepl/blob/7d2ed83b3a57f3883f4a9edd7f6cf83a8d0888b0/src/cider/nrepl/middleware/test.clj#L358

I think that’d be an easy change, so I’m not worried about this.

> Having worked on thread management, I’m now wondering if I missed a documentation on the execution model (not the implementation).

Your understanding is perfect.

I guess we never really documented this, so it’s now only an urban legend. 🙂

The spooky kind: passed amongst client implementors around campfires...

😄

Oh and one can’t interrupt a stray eval (an eval on a transient session) anymore

The difference between the behavior of eval and non-eval messages was a big argument in favour of moving most of the client functionality to middlewares, as if they constantly evaled some messages to get things like completion candidates that could impact performance negatively.

What’s a transient session?

transient session: non-cloned one

execution model: thanks for the background; what about serializing execution on session+op? (I don’t have an actual plan, just questioning the statu quo)

This will need to be reworked https://github.com/clojure-emacs/cider-nrepl/blob/7d2ed83b3a57f3883f4a9edd7f6cf83a8d0888b0/src/cider/nrepl/middleware/test.clj#L274-L287

> execution model: thanks for the background; what about serializing execution on session+op? (I don’t have an actual plan, just questioning the statu quo)

I guess it depends on the op - most ops don’t really affect the global state, so it’s fine to run them concurrently. I don’t see a need for serializing individual ops, unless something in the implementation of the op requires it - e.g. I can imagine multiple refresh ops running in parallel might be a bad idea, but I also wonder if that’s something people encounter in practice.

Perhaps there could be some flag to force ops to be serialized to make sure they’re safely applied?

> transient session: non-cloned one

Ah, got it. I’ve been wondering from time to time if we should allow the usage of non-cloned sessions at all. I don’t really see any value in them, but I might be missing something.

> This will need to be reworked https://github.com/clojure-emacs/cider-nrepl/blob/7d2ed83b3a57f3883f4a9edd7f6cf83a8d0888b0/src/cider/nrepl/middleware/test.clj#L274-L287

Yep. Not a big deal, though.

I’ve created a PR for #16 https://github.com/nrepl/nrepl/pull/98

@cgrand Thanks! I’ll take a look!