nrepl

https://github.com/nrepl/nrepl || https://nrepl.org
bozhidar 2018-12-12T10:18:11.097800Z

@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.

bozhidar 2018-12-12T10:19:14.098900Z

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.

bozhidar 2018-12-12T10:19:51.099800Z

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.

bozhidar 2018-12-12T10:22:53.100200Z

@dchelimsky The new Lein 2.8.2 is out! 🙂

cgrand 2018-12-12T10:25:22.101400Z

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

cgrand 2018-12-12T10:25:51.102Z

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

cgrand 2018-12-12T10:27:21.102500Z

Oh, that’s only for tests?

cgrand 2018-12-12T10:32:06.105Z

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 2018-12-12T10:38:13.105200Z

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

bozhidar 2018-12-12T10:38:17.105500Z

It’s not.

bozhidar 2018-12-12T10:39:15.106Z

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

bozhidar 2018-12-12T10:39:59.107100Z

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

bozhidar 2018-12-12T10:40:46.107300Z

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

bozhidar 2018-12-12T10:40:51.107500Z

Your understanding is perfect.

bozhidar 2018-12-12T10:41:16.108Z

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

cgrand 2018-12-12T10:43:55.109Z

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

bozhidar 2018-12-12T10:44:02.109200Z

😄

cgrand 2018-12-12T10:45:44.111Z

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

bozhidar 2018-12-12T10:46:08.111100Z

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.

bozhidar 2018-12-12T10:46:23.111400Z

What’s a transient session?

cgrand 2018-12-12T10:50:07.111900Z

transient session: non-cloned one

cgrand 2018-12-12T10:53:29.113700Z

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)

bozhidar 2018-12-12T11:09:14.114300Z

> 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)

bozhidar 2018-12-12T11:11:40.116500Z

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.

bozhidar 2018-12-12T11:12:30.117400Z

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

bozhidar 2018-12-12T11:12:39.117600Z

> transient session: non-cloned one

bozhidar 2018-12-12T11:13:35.118400Z

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.

bozhidar 2018-12-12T11:14:56.119Z

Yep. Not a big deal, though.

cgrand 2018-12-12T11:15:19.119300Z

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

bozhidar 2018-12-12T12:09:55.121200Z

@cgrand Thanks! I’ll take a look!