Revert "emacs: set FD_SETSIZE and DARWIN_UNLIMITED_SELECT on darwin"

[opsroller]

Reverts #391407

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

The changes merged introduce severe performance impacts when using emacs on battery. 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.

Another reason to not go the route of FD_SETSIZE

https://threadreaderapp.com/thread/1723398619313603068.html

[Patryk27]

Counterpoint - some other sources do provide FD_SETSIZE and people seem not to complain, e.g. https://github.com/d12frosted/homebrew-emacs-plus.

While I agree that raising the limit isn't the same as actually solving the issue, the solution-solution needs to happen upstream and until that happens, raising limit can be good enough and practical enough so that people don't end up frustrated.

[Patryk27]

Note that the DARWIN_UNLIMITED_SELECT flag is probably unnecessary, though - if anything, it should say _DARWIN_UNLIMITED_SELECT and since the patch works as-is, it implies that this flag doesn't affect anything here and can be removed.

[opsroller]

Note that the DARWIN_UNLIMITED_SELECT flag is probably unnecessary, though - if anything, it should say _DARWIN_UNLIMITED_SELECT and since the patch works as-is, it implies that this flag doesn't affect anything here and can be removed.

Can we connect at some point to actually step thru the issue? I'd really like to help you solve the frustrations, I too have dealt with them, but they usually pop up when in JavaScript/TypeScript and the issue is the actual Node server process that it spawns. It literally exhausts all system resources until you start getting popups about not being able to allocate memory for an application. My fear is that the way the emacs-lsp plugin is launching the LSP servers is making them children which is the likely underlying culprit.

[opsroller]

@Patryk27 So I just tested that theory, and it's exactly what's happening. When I launch any language server it spawns children, I suspect this may be the underlying cause.

[sielicki]

I would have no issues with the original PR if it was exposed as an override, even if it was opt-out rather than opt-in. But especially because it is applied unconditionally to every build variant under darwin, I agree this needs to be reworked.

[djgoku]

I would have no issues with the original PR if it was exposed as an override, even if it was opt-out rather than opt-in. But especially because it is applied unconditionally to every build variant under darwin, I agree this needs to be reworked.

I’ll consider an override, but after this conversation, I’m also exploring alternative methods to achieve my desired outcome without directly modifying nixpkgs or emacs-overlay.

[opsroller]

LMAO, Emacs is still using the old ass inotify apis, Apple migrated away from them years ago. So I am currently estimating the effort required to enable the new api's directly in Emacs, which would be usable as an overlay until the code is upstreamed.

[opsroller]

Ok, so I’ve written the foundation for the updated file notifier in emacs, it should have way better performance for watching files and doesn’t use any of the old interfaces, it properly uses dispatch, and so the actual file descriptor count drops to zero. Perhaps there’s a better place for this discussion. Anyone?

[jian-lin]

Let's merge this revert because

• emacs: set FD_SETSIZE and DARWIN_UNLIMITED_SELECT on darwin #391407 turns out to be controversial (even though no issue caused by it has shown)
• applying emacs: set FD_SETSIZE and DARWIN_UNLIMITED_SELECT on darwin #391407 in a user config is easier than reverting it.

[djgoku]

Let's merge this revert because

• emacs: set FD_SETSIZE and DARWIN_UNLIMITED_SELECT on darwin #391407 turns out to be controversial (even though no issue caused by it has shown)

• applying emacs: set FD_SETSIZE and DARWIN_UNLIMITED_SELECT on darwin #391407 in a user config is easier than reverting it.

I ended up with the below flake to implement what I need. Also the max amount of process I could create with this was less than 5k (for reference).

@jian-lin thanks again for all the help and sorry I caused you some extra work.

flake.nix

{
  inputs = {
    flake-utils.url = "github:numtide/flake-utils";
    emacs-overlay.url = "github:nix-community/emacs-overlay";
    nixpkgs.follows = "emacs-overlay/nixpkgs";
  };

  outputs = { self, flake-utils, emacs-overlay, nixpkgs }:
    flake-utils.lib.eachDefaultSystem (system:
      let
        pkgs = import nixpkgs {
          inherit system;
          overlays = [ emacs-overlay.overlay ];
        };
      in rec {
        packages.default = (pkgs.emacs-git-pgtk.override { }).overrideAttrs (old: rec {
          NIX_CFLAGS_COMPILE = (old.env.NIX_CFLAGS_COMPILE or "")
            + pkgs.lib.optionalString pkgs.stdenv.hostPlatform.isDarwin
            " -DFD_SETSIZE=10000 -DDARWIN_UNLIMITED_SELECT";
        });
      }
    );
}