clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.52k stars 643 forks source link

Should cider work with `--with-poll` emacs? #3455

Open jmckitrick opened 10 months ago

jmckitrick commented 10 months ago

There's a newish feature in emacs that swaps out select for poll to allow over 1024 file descriptors to be open, IIUC. It's supposedly a 'drop in' replacement. However, my first attempt to run emacs and cider with this option resulted in a hung connection during 'jack in'. Should this feature even work with cider at all? Or is it worth investigating?

vemv commented 10 months ago

Thanks for bringing attention over this!

We could expand our CI matrix and see which combinations succeed/fail.

(There aren't a lot of 'jack in' integration tests, but maybe a more basic test fails, giving us a hint)

jmckitrick commented 10 months ago

I build and run emacs HEAD every week and tinker with settings, so I'm happy to help with this.

vemv commented 9 months ago

I'll try out https://github.com/d12frosted/homebrew-emacs-plus which makes this option easy, apparently

(TIL about that distro)

jmckitrick commented 9 months ago

@vemv I use that one and https://github.com/daviderestivo/homebrew-emacs-head

vemv commented 9 months ago

I ran brew install emacs-head@30 --with-cocoa --with-poll and could repro the issue.

By running M-x toggle-debug-on-error and then cider-connecting I could see

[nREPL] Establishing direct connection to 127.0.0.1:64844 ...
[nREPL] Direct connection to 127.0.0.1:64844 established
  signal(error ("Sync nREPL request timed out (op clone id 1 time-stamp 2023-09-26 16:32:21.439826000) after 10 secs"))
  error("Sync nREPL request timed out %s after %s secs" ("op" "clone" "id" "1" "time-stamp" "2023-09-26 16:32:21.439826000") 10)
  nrepl-send-sync-request(("op" "clone" "id" "1" "time-stamp" "2023-09-26 16:32:21.439826000") #<buffer *cider-uninitialized-repl*> nil nil)
  nrepl-sync-request:clone(#<buffer *cider-uninitialized-repl*>)
  nrepl--init-client-sessions(#<process nrepl-connection>)
  nrepl-start-client-process("127.0.0.1" 64844 nil #f(compiled-function (_) #<bytecode 0xf69663072ad1ccf>) nil)
  cider-nrepl-connect((:host "127.0.0.1" :port 64844 :project-dir "/Users/vemv/orchard/" :repl-init-function nil :session-name nil :repl-type clj))

This would require some investigation.

Perhaps the Emacs 30 changelog explains an adoption guide? i.e. the transition steps.

jmckitrick commented 9 months ago

@vemv I saw exactly the same thing. Yet every article I read on 'poll vs select' claimed it was a drop-in replacement that should be transparent to the user. At least, that was my impression.

I'll take a closer look at the emacs docs, but I'm pretty sure there was no migration guide or something similar.

vemv commented 9 months ago

It might be even an unofficial patch https://github.com/daviderestivo/homebrew-emacs-head/blob/master/patches/0011-Poll.patch - do you know where it comes from?

jmckitrick commented 9 months ago

@vemv Though I wouldn't be surprised if just a handful of simple changes in cider-nrepl solve the entire issue.

vemv commented 9 months ago

Those would be welcome as long that nothing was impacted for the vanilla ~patch~ path.

jmckitrick commented 9 months ago

@vemv The patch is for the macos build... but the feature itself is in the underlying emacs. Let me see if I can find it.

jmckitrick commented 9 months ago

But you're right.... this patch is more extensive than I initially thought. Maybe I'll try a vanilla terminal mode emacs and see if that works.

jmckitrick commented 9 months ago

@vemv I found the source of the patch!

https://lists.gnu.org/archive/html/emacs-diffs/2022-05/msg00142.html

I was mistaken in thinking this was in emacs master. It's actually on a feature branch. I wonder where the discussion would happen, if this is slated to become part of emacs.

vemv commented 9 months ago

Nice!

I can't promise we'll give this a try super soon (other than a quick look and trial/error - already done).

But leaving the issue open seems reasonable, for visibilty. I hear that this capability would be benefitial for LSP performance. OTOH, there's more than one LSP offering for Emacs itself (eglot, lsp-mode), so it wouldn't surprise me if the existent ones are good-enough for the vast majority of cases.

Which is to say, sometimes it's wise to stick to what the authors wanted us to use.

Note that https://lists.gnu.org/archive/html/emacs-diffs/2022-05/msg00142.html is a mailing list item. You can certainly try bumping that conversation or perhaps contacting the author.

jmckitrick commented 9 months ago

@vemv I emailed him yesterday. I'll report back what I find out.