d12frosted / homebrew-emacs-plus

Emacs Plus formulae for the Homebrew package manager
MIT License
2.38k stars 187 forks source link

add --with-mps option #707

Closed nailuoGG closed 3 months ago

nailuoGG commented 3 months ago

build with MPS garbage collector, using scratch/igc branch as default #706

brew install emacs-plus@31 \
    --with-mps \
    --with-imagemagick  \
    --with-native-comp \
    --with-modern-alecive-flatwoken-icon

tip:

set --with-mps will use scratch/cgi as default branch.

set HOMEBREW_EMACS_PLUS_31_BRANCH=scratch/igc if merged to other branch

nailuoGG commented 3 months ago

Thanks for your review.

But most importantly, I'd love to understand what this feature is about and why should we increase complexity of the formula for this feature. Over last few years I am trying to remove number of build options :)

Feature: Improve Emacs' garbage collection (GC) performance by replacing its built-in GC with a better one from Ravenbrook/mps.

Why: Experiment with improved GC performance, more testing needed to make it production-ready. Also simplifies macOS setup and makes it easy to test MPS.

What is the state of its development? And is there a consensus (link needed) that it's going to be merged to master? Or is it simply experimental thing?

Status: This feature is still experimental, no plans to merge into the master branch soon.

d12frosted commented 3 months ago

Improve Emacs' garbage collection (GC) performance by replacing its built-in GC with a better one from Ravenbrook/mps.

Thanks for explanation.

Why: Experiment with improved GC performance, more testing needed to make it production-ready. Also simplifies macOS setup and makes it easy to test MPS.

That makes sense.

Status: This feature is still experimental, no plans to merge into the master branch soon.

I see.

Frankly, I am not convinced that adding an extra option to emacs-plus would help developers somehow. Building from separate branch is as trivial as brew edit emacs-plus@31. We can simply share a gist with edits necessary. But considering the experimental nature of the feature it might just increase maintenance cost. I can not provide any kind of support for this feature anyways. And having it as an option in emacs-plus (and especially part of build matrix on CI) is a signal that we do support it or at least try to, which is not true.

So I propose to prepare a patch and share it as a gist in #706. If we get enough traction and/or the feature becomes more stable/closer to release - we can add an extra option.

cc @aaronjensen as you have some healthy thoughts on maintenance :)

aaronjensen commented 3 months ago

One could also look up the specific revision and use the existing HOMEBREW_EMACS_PLUS_31_REVISION option.

Given the experimental nature of this, and its existence upstream (so that it can easily be opted into already), I'd be inclined to not complicate the build by allowing building from two separate branches. It could even become possible that the existing patches do not apply to the mps branch, and then what?

Personally, I wouldn't be terribly interested in trying this feature because gcmh has meant that I haven't seen GC hangs in years. I think it's great that someone is working on it, however.

nailuoGG commented 3 months ago

So I propose to prepare a patch and share it as a gist in https://github.com/d12frosted/homebrew-emacs-plus/issues/706. If we get enough traction and/or the feature becomes more stable/closer to release - we can add an extra option.

I agree that keeping things simple is a good principle, and creating a separate gist for others to use makes sense. 😄

For users, the performance boost from replacing GC might not be immediately noticeable, especially when compared to using something like gcmh.

d12frosted commented 3 months ago

At least I am happy that we have a reference for those willing to build with MPS. Let's see how development of this feature evolves and we can reconsider adding this option later.