persp-maybe-kill-buffer is too slow

[gcv]

I had to comment out persp-maybe-kill-buffer use in kill-buffer-query-functions. See https://github.com/nex3/perspective-el/commit/3e4efaba598bb01834bbc4fbeb3e1d7b48579624.

persp-maybe-kill-buffer is pretty slow since it does a lot of traversing across perspectives and buffers. And it gets called a lot, especially with some more sophisticated packages like Helm and Magit that create and destroy buffers all the time.

This is possibly related to the problem in #167. Because I removed some expected behavior, tests are now failing.

@mehw: Could you please look into fixing this? Thanks!

[gcv]

I think there needs to be a much faster mechanism for checking which perspectives a buffer is in. Maybe each buffer should keep track of it in a buffer-local variable, which will make the lookup in persp-maybe-kill-buffer much faster.

[mehw]

I think there needs to be a much faster mechanism for checking which perspectives a buffer is in. Maybe each buffer should keep track of it in a buffer-local variable, which will make the lookup in persp-maybe-kill-buffer much faster.

Thank you for the suggestions @gcv, I'm starting debugging to focus the problem... I'll report back to you ASAP ;)

[mehw]

Hi @gcv, after nights of debugging I can't manage to trigger the mentioned bug... maybe @deejayem, @ibytao, and @dpassen can give some context. I would like to design some regression tests...

I see there was a discussion about the recursive minibuffer in #163... I tried (setq enable-recursive-minibuffers t) and also (savehist-mode) with (setq savehist-autosave-interval 1) but nothing...

How should I setup Emacs 27.2 to reproduce the bug #167? I'm on Gentoo GNU/Linux, by the way.

What sequence of commands should I execute?

Thank you all. In the meantime, I'll keep trying ;)

[gcv]

Thanks for looking into it. Don’t worry about that bug specifically. Commit https://github.com/nex3/perspective-el/commit/6e87eeb6160345390406a82af0db61782bfb9913 in master worked around it.

I commented out persp-maybe-kill-buffer because of how frequently it gets called. It made basic Magit use feel very sluggish. When I put a breakpoint in it and start Magit or Helm, persp-maybe-kill-buffer gets hit a lot because those packages create and destroy tons of temporary buffers. I run with a lot of perspectives and buffers, so the traversals in that function take a while. Feels like there’s an O(n^2) thing going on.

Maybe just adding a simple heuristic would be enough. Like assuming every buffer name that starts with a space (“hidden” buffer) can automatically be killed because none of them belong to a perspective. Not sure.

I still really like the intent behind persp-maybe-kill-buffer — it’s just a fairly intrusive thing to have to run all the time, and so we need to make it really fast.

Feel free to look at my Emacs configuration (27.2, Mac) for inspiration: https://github.com/gcv/dotfiles/tree/master/emacs

[gcv]

@mehw: I put in a hack to allow killing temporary buffers (which we should not care about) without the expensive check. See 672d02dde0c2f8c951e3032e21050e1375a2692e.

Then I reenabled the check, but hid it behind a feature flag, and turned it off by default: 248b4e96400b64cbaf8a431c437e6424a7665624 — tests are now passing again.

I'll try running the code with persp-feature-flag-prevent-killing-last-buffer-in-perspective set to t for a while and see if I run into trouble. Please do the same. If everything looks good, I'll turn it on by default.

[mehw]

@gcv: Thank you. I'll take a look this week-end. I'm sorry for answering you only now... crazy weeks ;)

[mehw]

@gcv: Hi, I managed to speed up the processing further... I'm currently perfecting the code. Expect a PR soon ;)

I think there needs to be a much faster mechanism for checking which perspectives a buffer is in. Maybe each buffer should keep track of it in a buffer-local variable, which will make the lookup in persp-maybe-kill-buffer much faster.

Benchmarks reveal that the processing could be sped up going bare metal, beyond the utility functions, accessing directly the data.

I put in a hack to allow killing temporary buffers (which we should not care about) without the expensive check. See 672d02d.

It's working on temporary buffers killed directly with kill-buffer. Smart solution ;)