Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix over consing #152

Merged
merged 6 commits into from
Jun 28, 2019
Merged

Fix over consing #152

merged 6 commits into from
Jun 28, 2019

Conversation

Zulu-Inuoe
Copy link
Contributor

This fixes #149

The main fix is to not calculate the list of 'other buffers with same purpose' unless we're going to be replacing the buffer. This prevents calculating said list for buffers that aren't being displayed on any window, in particular, background buffers created with with-temp-buffer and the like.

Additionally, minor efficiency improvements for calculating buffer purpose are included

Using `cl-block` combined with `maphash` prevents creating several intermediate lists of which we only want one item.
Note- `purpose--iter-hash` is no longer used, but still updated to lower consing by simply using `push` rather than appending new entries to the tail of the already-gathered-results
Fixes bmag#149
The problem is that 'gathering a list of other buffers with the same purpose' conses due to an inevitable maphash later on, and  even though it's not a ton, it adds up quick because this function is called on every kill-buffer call. Even for the tons and tons of background temp buffers used by company, helm, sly, and others.

The fix: We don't need to figure out the list of other buffers unless we actually come across a window that was displaying the killed buffer. So this fix just delays calculating it until we actually need it.
@coveralls
Copy link

coveralls commented Jun 17, 2019

Coverage Status

Coverage decreased (-0.03%) to 69.951% when pulling fbbb370 on Zulu-Inuoe:fix-over-consing into fb649bb on bmag:master.

@mfiano
Copy link

mfiano commented Jun 26, 2019

I look forward to this being merged. Emacs slows to a crawl after a short while without this.

Copy link
Owner

@bmag bmag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Most of it is good, except for purpose--buffer-purpose-mode where your changes introduce a bug. There are a few minor details that I'd like to fix, and then I will merge.

window-purpose-core.el Outdated Show resolved Hide resolved
window-purpose-core.el Show resolved Hide resolved
window-purpose-core.el Show resolved Hide resolved
window-purpose-utils.el Show resolved Hide resolved
window-purpose-x.el Show resolved Hide resolved
window-purpose-x.el Show resolved Hide resolved
@Zulu-Inuoe
Copy link
Contributor Author

Zulu-Inuoe commented Jun 28, 2019

Sorry about that! e935c88 is broken because I didn't quote the found catch tag. Pushed a new fix

@bmag bmag merged commit f642196 into bmag:master Jun 28, 2019
@bmag
Copy link
Owner

bmag commented Jun 28, 2019

Thanks a lot! Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Large memory usage when combined with company
4 participants