emacs: set FD_SETSIZE and DARWIN_UNLIMITED_SELECT on darwin

[djgoku]

This fixes the issue with macOS and too many open files. Before this patch on macOS this would (shell-command-to-string "ulimit -n") return 1024 and only a few hundred process would complete:

;; Taken from https://en.liujiacai.net/2022/09/03/emacs-maxopenfiles/
(shell-command-to-string "ulimit -n")
;; 10000

(dotimes (i 2000)
 (make-process
  :name (format "Sleep-%s" i)
  :buffer nil
  :command '("sleep" "60000")
  :connection-type 'pipe))

with this patch I get 9472 returned for (shell-command-to-string "ulimit -n") I started 4000 which was more than enough for my testing.

Prior work:

d12frosted/homebrew-emacs-plus@4b34ed7

Context:

https://en.liujiacai.net/2022/09/03/emacs-maxopenfiles/

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[jian-lin]

I tested this PR and it works. Thanks!

[djgoku]

Thank you for the guidance and helping me get to this point!

[opsroller]

This is a joke, right? Never have I ever experienced such an error. And I use emacs in 90% of my daily activities across dozens of codebases including emacs.

This is the correct way, https://neo4j.com/developer/kb/setting-max-open-file-limits-on-osx/

[opsroller]

emacs-lsp/lsp-mode#1775 (comment)

[jian-lin]

@opsroller

This is a joke, right?

Please be kind and adhere to our Code of Conduct.

This is the correct way, https://neo4j.com/developer/kb/setting-max-open-file-limits-on-osx/

IIUC, this setting only affects daemons, not user agents. So it has no effect if a user opens a GUI Emacs.

Quoting your reverting PR

This should be handled via proper MacOS apis, not some inline hack.

What proper MacOS apis do you have in mind? Please share it as it will help users having this "too many open files" issue.

The changes merged introduce severe performance impacts when using emacs on battery.

What is the severe performance impacts? How to reproduce it? If the issue is reproducible, we should revert this PR.

What channel do you use? I ask this because this PR has not yet reached any channels at the time of your writing. This makes me wonder if the severe performance impact is caused by this PR.

The FD_SETSIZE is a cheap way to fix the issues that few people have encountered when using Emacs + LSP, which points to the fact that the issue should be solved directly within the Emacs codebase and not NixPkgs.

I do not follow. The issue is a low limit of open file number imposed by darwin. Why should / How can it be fixed in Emacs codebase?

EDIT: Ah, maybe you mean make Emacs not use select on darwin.

Another reason to not go the route of FD_SETSIZE https://threadreaderapp.com/thread/1723398619313603068.html

As I said before, I know nothing about darwin. However, a quick search tells me that darwin select is different so your link may not apply.

[djgoku]

@jian-lin I am fine with reverting. I have a path forward using what you helped me with here.

nix-community/emacs-overlay#480

[opsroller]

@jian-lin @djgoku Sorry for the intensity, Emacs is literally my lifeline. The real issue is the devil hidden in the details, the reasoning behind the FileDescriptor limits is very well thought out. If this limit is expanded to infinity, what is there to prevent a full on overflow of FileDescriptors, there is a long history of using these kinds of surfaces for attack purposes, as well as performance going to hell.

To test and fully understand the issue that you are experiencing it would be wise for you to install Xcode and Sign your emacs bundle 'Emacs.app', which you can then open the Instruments application and do a performance deep dive, and it will expose where the exact issue is. I'd be willing to help with this, but I don't see how we could possibly integrate that as an automated step directly within nixpkgs. But I do recognize what problems you're encountering and why you want to attack the issue with the aforementioned patch.

I am going to figure out the steps for profiling, and I will post them somewhere accessible and link back to here.

[jian-lin]

I have second thoughts about the merge. Maybe it should have be reviewed by some darwin experts before being merged. I am slightly for reverting it now.

Before that, I would like to know some facts about "severe performance impacts". Please answer these questions.

The changes merged introduce severe performance impacts when using emacs on battery.

What is the severe performance impacts? How to reproduce it? If the issue is reproducible, we should revert this PR.

What channel do you use? I ask this because this PR has not yet reached any channels at the time of your writing. This makes me wonder if the severe performance impact is caused by this PR.

[opsroller]

@jian-lin Here's one example of why the ulimit is set to 1024 by default.

https://unix.stackexchange.com/questions/37156/fork-bomb-on-a-mac

WARNING: DO NOT RUN THIS BLINDLY

:() { #Define a new shell function
  :|:& #Pipe function named ':' through itself, creating two copies of itself, and make them run in the background
} #End of function definition block
;: #Call the ':' function. Note how the function is defined with two calls to itself piped through each other. This starts a chain reaction: those two copies will in turn create two more, and so on, ad infinitum

[jian-lin]

Here's one example of why the ulimit is set to 1024 by default.

https://unix.stackexchange.com/questions/37156/fork-bomb-on-a-mac

This link (fork boom) is not relevant here because this PR does not change ulimit.

@opsroller You repeatedly avoid directly answering these questions about "severe performance impacts". This PR did not reach any channels at the time of your writing. You keep posting unrelated links. After seeing all these, I have to admit that I believe this PR does not "introduce severe performance impacts when using emacs on battery".

[opsroller]

ulimit controls the the File Descriptor limits, which is what you're trying to accomplish, no?

[jian-lin]

ulimit controls the the File Descriptor limits, which is what you're trying to accomplish, no?

Yes and yes. But changing ulimit alone is not enough. To actually increase the limit, you have to change ulimit and apply this PR at the same time. ulimit and this PR are orthogonal.

[Patryk27]

This is a joke, right? Never have I ever experienced such an error.

I have, many times in the past when working on larger (thousands of files) lsp-based projects in Emacs.

The poll patch is the better approach, but some people report problems with its stability, so bumping FD_SETSIZE is the most practical approach for the time being, IMO.

[Patryk27]

Couple of other counterpoints:

If this limit is expanded to infinity, [...]

Ok, but this commit doesn't expand the limit to infinity.

what is there to prevent a full on overflow of FileDescriptors [...]

I mean, the limit itself? Not sure I follow as this commit doesn't remove any limit whatsoever.

there is a long history of using these kinds of surfaces for attack purposes, as well as performance going to hell.

I'd like to see some specifics as well, for both claims.

and it will expose where the exact issue is

The issue is that Emacs (well, lsp-mode) is trying to create too many file watchers; it's a known issue.

[opsroller]

@Patryk27 I currently have NixPkgs open in a project within emacs30. I have LSP running, and it's throwing zero errors anywhere, and LSP is working perfectly. This is before the patch is applied. I suspect that you have an error with the language server itself and not emacs, unless this patch is strictly for building emacs within llvm in which you should be using _DARWIN_UNLIMITED_SELECT instead of DARWIN_UNLIMITED_SELECT as per apple's documentation.

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/select.2.html

[Patryk27]

and LSP is working perfectly.

Well, in that case your lsp client simply isn't monitoring changes to the filesystem (or the monitoring happens outside of Emacs, both of which are mildly out-of-the-specification bad situations).

There's simply no way for vanilla Emacs to monitor more than 1024 files on MacOS, so the only answer to "why it works on my machine" is "it doesn't either, you just must've not noticed".

I suspect that you have an error with the language server itself and not emacs

This is not a "me problem" - if you Google for emacs macos too many files you'll find tons of people saying the same stuff, across different lsp clients (rust-analyzer in my case, though).

[opsroller]

On another note, the main disagreement is "Why isn't this being submitted directly upstream to the emacs project for review?". If it's an emacs issue why are we patching it via Nix and not involving the entirety of the emacs community who actually have a way deeper understanding of what's at play?

[Patryk27]

The Emacs community is involved, see e.g. https://lists.gnu.org/archive/html/help-gnu-emacs/2023-10/msg00022.html (iirc there's also a somewhat newer thread with someone trying to resuscitate that patch).

But, as always, there's the ideal solution and then there's the practical solution (for the time being) 😅

[opsroller]

and LSP is working perfectly.

Well, in that case your lsp client simply isn't monitoring changes to the filesystem (or the monitoring happens outside of Emacs, both of which are mildly bad situations).

There's simply no way for vanilla Emacs to monitor more than 1024 files on MacOS, so the only answer to "why it works on my machine" is "it doesn't either, you just must've not noticed".

I suspect that you have an error with the language server itself and not emacs

As I said, this is not a "me problem" - if you Google for emacs macos too many files you'll find tons of people saying the same stuff, across different lsp clients (rust-analyzer in my case, though).

My LSP Server is a completely different process than emacs, which is exactly why I think this patch is not up to snuff.

And finally, Emacs is NOT LSP, so why we're spaghettifying the solution is whats got me tripping.

[Patryk27]

My LSP Server is a completely different process than emacs, which is exactly why I think this patch is not up to snuff.

Yes, but watching has to (should?) happen from within Emacs - IIRC that's how LSP is designed (IDE presents sort of a virtual file system to the underlying server, the underlying server doesn't try to access files on its own).

[opsroller]

My LSP Server is a completely different process than emacs, which is exactly why I think this patch is not up to snuff.

Yes, but watching has to (should?) happen from within Emacs - IIRC that's how LSP is designed (IDE presents sort of a virtual file system to the underlying server, the underlying server doesn't try to access files on its own).

Question, if you can only edit one file/buffer at any single moment then why would you need to watch all of the other files? Is some external process changing/generating them? It seems really like a misconfiguration on your end.

Also, I'm dying to see your LSP configuration to test it.

[jian-lin]

To be honest, I am also curious about why Emacs+Lsp needs Emacs to open many fds. A quick search leads me to this: https://lists.gnu.org/archive/html/emacs-devel/2022-05/msg00158.html

[opsroller]

@jian-lin I must also say, the way MacOS/Darwin handles filenotify is disgusting and they never really fixed it's flaws, unless you dig into Grand Central Dispatch, but it's all a mess.

[sielicki]

just want to make sure you two aren't talking past each other -- it's not obvious to me whether everyone in this thread is running the same release of darwin emacs. I see at least one reference to emacs30 which implies not macport.

In general, I overall agree that this patch should not have gone in without at least being exposed behind an override, but the emacs experience is full of collecting one-weird-trick hacks like this to make it tolerable. eg: needing a plugin to make scrolling not suck, needing magic garbage collector hacks, an aarch64 nixpkgs build that was unusable and crashing for two years with a lot of eyes and nobody able to spot what was wrong, list goes on and on. Whether or not this hack actually changes anything for the better or not, it's definitely on-character. So I understand the frustration, but also can't really fault anyone.