Conversation
github-actions
bot
added
6.topic: emacs
ofborg
bot
added
the
8.has: clean-up
ofborg
bot
added
10.rebuild-darwin: 11-100
|
Result of 2 packages built:
|
bobby285271
added
the
12.approvals: 2
alyssais
added
the
2.status: merge conflict
jian-lin
force-pushed
the
emacs-native-comp
branch
from
d94146a to
0496970
Compare
jian-lin
removed
the
2.status: merge conflict
bobby285271
removed
the
12.approvals: 2
There was a problem hiding this comment.
Why do we want this? Wouldn't it have performance benefits to leave this enabled?
There was a problem hiding this comment.
When someone has lots of packages from ELPA, then after launching Emacs it will try to compile all of these packages which is quite heavy on CPU. I agree with the author of the PR that it is better to make the native compilation for user packages opt-in.
There was a problem hiding this comment.
Deleting this line (setq native-comp-deferred-compilation nil) does have performance benifits but users will need some compilation if they install packages not from nixpkgs. (You can see the release notes for more information about this).
Some users don't want that compilation:
@wahjava wrote in #168076 (comment)
it uses 100% CPU (in its default configuration) when you install any new packages for native compilation
I set native-comp-deferred-compilation to nil in this pr to have less impact on users. However, I would love to remove this line (setq native-comp-deferred-compilation nil) and instead add instructions that users don't want this can set (setq native-comp-deferred-compilation nil) in their init file, like what @axelf4 said and @adisbladis agreed in #168076:
Would not those users be better off disabling just-in-time native compilation in their init files, while still benefiting from the packaged Emacs distribution being ahead-of-time compiled?
What do you think?
There was a problem hiding this comment.
I think I prefer not setting this. Everything else in this file looks like me to distro-compatibility code, which I think is a lot more appropriate for us to decide on, as opposed to user preferences, which I think should be left to upstream (for defaults) and, well, the users :)
I think removing it but suggesting it to users in the release notes would be good.
There was a problem hiding this comment.
Regarding upstream's default, one may argue that upstream does not enable native-comp by default. However, since this change is mentioned in the release notes, it should be fine.
@lheckemann What you suggested has been done.
|
Also, I've had problems building emacs packages on aarch64 in the past, package builds would crash. Let's run a couple packages through ofborg to make sure this is fixed, and if not only enable it on x86 by default. @ofborg build emacsPackagesNg.magit emacsPackagesNg.counsel emacsPackagesNg.vterm |
|
That looks fine, so that only leaves the async question :) the reason should probably be documented in the release notes which we already have. |
jian-lin
force-pushed
the
emacs-native-comp
branch
from
0496970 to
51bd7cf
Compare
jian-lin
changed the title
|
Hm, I'm sure I made another comment in here somewhere improving the wording of the release notes. Never mind though, let's just merge this and get the functionality in. |
The default Emacs package uses native-comp by default as of NixOS/nixpkgs#176780
6 participants
Description of changes
cc @adisbladis
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes