bnbeckwith / wc-mode

Wordcount minor mode for Emacs
68 stars 11 forks source link

wc-mode slows emacs down #13

Closed obrowny closed 4 years ago

obrowny commented 6 years ago

Hello, wc-mode is very nice but with a 60 000 words buffer, it becomes unusable as emacs becomes very laggy. I don't know how the package is working, but it seems to scan the whole buffer permanently. I don't know if the package could take advanvage of the async function and if it would increase performance. thanks

sten0 commented 5 years ago

@bnbeckwith, what do you think about using run-with-idle-timer so that words are only counted during pauses? This will reduce resource usage. It also seems like it would be nice to have a defcustom that if nil means that wc-mode will live-update (current behaviour).

Then to defend against wc during idle becoming forced breaks from work, using jwiegley's async.el. Does this sound like an acceptable dependency? It has better backwards compatibility than the new generators (Emacs 25 ) and threads (Emacs 26).

@obrowny is the "async function" you're referring to from that async.el, or from somewhere else?

sten0 commented 5 years ago

P.S. run-with-idle-timer also has this neat side effect: things changing in the modeline are distracting, and this limits the distracting output to idle periods :-)

bnbeckwith commented 5 years ago

@sten0, I’m fine with that approach.

If you make a pull request, I’ll be happy to review it. If you want me to make the changes, expect it to take some time before I can get something in place.

sten0 commented 5 years ago

@bnbeckwith, done, we're half-way there :-)

@obrowny, would you test to see if there's any improvement yet 60 000 word files? I plan to add async support soon, but please ping me if I seem to be taking too long--if that happens it's likely I ran out of free time and forgot, so will need a reminder.

sten0 commented 5 years ago

Some notes on #14: With the timer set to "0" it slows down Emacs even when wc-mode isn't running, I think because those functions are autoloaded, which set up the idle timer, the evaluation of the expression bound to it, and consequently runs wc-count all the time, even when wc-mode isn't running. Yeah, not great...

I've started working with async.el and discovered that modifying wc-mode to use it will require the use of async-inject-variables. This, in combination with my current run-with-idle-timer is starting to feel like it's becoming a bit spaghetti-code and difficult to understand. Also, I think the modeline eval loop is per-buffer, and so the wc-mode on your master branch only runs wc-count for active buffers, where my idle-timer approach would need to add a conditional for this (which would slow things down).

Consequently I'm starting to think that it might be better to rethink the plan, because run-with-idle-timer isn't as flexible as I expected it to be, and think we'll all agree that simplicity and readability should be prioritised.

@bnbeckwith, if I go with a pure async.el solution, then I think I'll still need to make some minor design changes to wc-mode, plus adding some kind of lock, because if the modeline evaluates in a loops (as I now suspect it does) then we'll end up with a potentially infinite queue of async wc-count requests without this ;-)

It's a fun project; thank you for the opportunity to work on it :-)

sten0 commented 5 years ago

Update: progress stalled do to lack of free time, but I will continue working on making wc-mode asychronous as soon as I have some free time.

@obrowny, would you please test #14?

sten0 commented 5 years ago

Progress update:

And I have a 3rd and 4th item for substantial gains. The 4th was a hint from someone who I'll properly credit with a "thank to X for the hint" comment :-)

sten0 commented 5 years ago

P.S. I deleted the spaghetti code and rewrote

bnbeckwith commented 5 years ago

@sten0, it seems that you have four items/improvements. Would it make sense to have 4 PRs so they can be checked and completed one at a time (independently)?

sten0 commented 5 years ago

@bnbeckwith, well, they're more logical sub-goals. V2 of #14 (only includes item #1, the built-in Emacs idle timer) uses an integrated approach that wouldn't have been possible if I hadn't first learned about all the potential pitfalls as I worked through it. V2 of #14 also necessarily integrates #1 and #2.

Here is the 10% that remains for V2:

  1. Cleanup any dangling async and timer processes.
  2. Investigate conditional firing of the idle timers, because it still introduces a minimum latency of 1x the idle-wait-timer-value, and the average modeline update latency is 2x this value.
  3. Deleting my verbose debugging instrumentation ;-)

@bnbeckwith, how do you want to approach backwards compatibility? Do you want all users to move to the new async code-path with a 0 or nil idle-timer delay, or do you want to retain the old functions and add a defcustom so the user can choose their preferred codepath? (eg: retain the old synchronous behaviour)

agent18 commented 4 years ago

@bnbeckwith wc-mode seems to be last committed on 27 Jan 2017. Would it be possible to release newer updates that @sten0 made, i.e., the "improvement" using run-idle-timer?

Thanks.

bnbeckwith commented 4 years ago

Yes, I can release the first PR into the mainline.

bnbeckwith commented 4 years ago

This code was released as part of v1.4.

@sten0 Thank you for your hard work.

agent18 commented 4 years ago

Hi thank you for the release. It's much better than before. But every five lines I type it gets slow again. Here is a video of what I mean: https://photos.app.goo.gl/nYs2QYhF1sxSGVqo8

vyorkin commented 4 years ago

It is too slow for me too. Quick profiling shows that wc-count gets called too often. Sorry, I didn't read the whole issue

bnbeckwith commented 4 years ago

@vyorkin @agent18, I'm sorry that you are still having issues. A couple of things:

  1. Have you tried staring up emacs without an init file and turning on just wc-mode? (emacs -q)
  2. How are you turning it on?
  3. Are you configuring anything special?

I really want to make this work for you all and uncover any issues.

vyorkin commented 4 years ago

Thanks for a quick response!

  1. I've just tried testing it in isolation using only the following config (you could use it to reproduce the issue): http://ix.io/2e3Q (emacs -q --load test-wc-mode.el init.org, where test-wc-mode.el is the gist I've provided) 2, 3. No, it is just (use-package wc-mode)

I tried to enable wc-mode on a pretty big org-file (my huge 8000 lines config) and run the profiler: profiler-run, choose cpu, then scroll with C-n, C-p for some time, then profiler-report. 2020-03-12_20-53