bug#69703: Ispell process restarted too often

bug#69703: Ispell process restarted too often

[Juri Linkov]

When different projects have different personal dictionaries
and flyspell mode is used, then ispell process is killed and restarted
almost on every key press.  This creates hundreds of hunspell processes
that soon fill all available memory.  Is it possible to fix this?

Here is an example of project-local .dir-locals.el:

  ((nil . ((ispell-personal-dictionary . "/tmp/test/.hunspell")
           (mode . flyspell))))

Example of backtrace after switching buffers
with different personal dictionaries:

  ispell-kill-ispell(t)
  ispell-internal-change-dictionary()
  ispell-buffer-local-dict()
  ispell-accept-buffer-local-defs()
  flyspell-accept-buffer-local-defs()
  flyspell-word()
  flyspell-check-changes()

Also tried to make the ispell process buffer-local with:

  (make-variable-buffer-local 'ispell-process)
  (make-variable-buffer-local 'ispell-current-personal-dictionary)

But this is worse and caused more problems:

  Error enabling Flyspell mode:
  (Buffer  *temp* has no process)
    flyspell-mode()
    set-auto-mode-0(flyspell-mode t)
    hack-one-local-variable(mode flyspell)
    hack-local-variables-apply()
    hack-dir-local-variables-non-file-buffer()
    project--value-in-dir(project-vc-name "/tmp/test/")
    project-name((vc Git "/tmp/test/"))
    project-mode-line-format()
    eval((project-mode-line-format) t)
    redisplay_internal\ \(C\ function\)()

bug#69703: Ispell process restarted too often

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 10 Mar 2024 09:37:20 +0200
> 
> When different projects have different personal dictionaries
> and flyspell mode is used, then ispell process is killed and restarted
> almost on every key press.  This creates hundreds of hunspell processes
> that soon fill all available memory.

If the speller process is killed each time before starting a new one,
how come there's more than just one running at any given time?  IOW,
how come you have "hundreds of hunspell processes" in memory?  Does
killing a hunspell process fail for some reason?

And why are you saying "almost on every key press"? are different
personal dictionaries defined for different portions of text in some
way?  Otherwise, I'd expect hunspell to be restarted when you change
buffers, whuch is hardly "on every key press".  Or what am I missing?

> Is it possible to fix this?

Not easily, AFAIR.  But I don't think I understand the nature of the
problem you are experiencing, see above.

bug#69703: Ispell process restarted too often

[Juri Linkov]

>> When different projects have different personal dictionaries
>> and flyspell mode is used, then ispell process is killed and restarted
>> almost on every key press.  This creates hundreds of hunspell processes
>> that soon fill all available memory.
>
> If the speller process is killed each time before starting a new one,
> how come there's more than just one running at any given time?  IOW,
> how come you have "hundreds of hunspell processes" in memory?  Does
> killing a hunspell process fail for some reason?

Actually, hundreds were when I tried to use buffer-local ispell-process
because then ispell-internal-change-dictionary didn't kill ispell-process
but ispell-init-process created new processes.

But with the default configuration there are "only" a dozen of processes.
I guess one process per buffer.  And this is even in case when ispell-process
is NOT buffer-local.  I noticed that all these processes are created
after loading the desktop buffers.

> And why are you saying "almost on every key press"? are different
> personal dictionaries defined for different portions of text in some
> way?  Otherwise, I'd expect hunspell to be restarted when you change
> buffers, whuch is hardly "on every key press".  Or what am I missing?

On every key press was only when trying to use buffer-local ispell-process.

But with the default configuration ispell-process is killed and started
"only" on switching buffers that is very annoying since most of the time
I see these messages in the echo area:

  Ispell process killed
  Starting new Ispell process /usr/bin/hunspell ...
  Ispell process killed
  Starting new Ispell process /usr/bin/hunspell ...
  Ispell process killed
  Starting new Ispell process /usr/bin/hunspell ...
  ...

>> Is it possible to fix this?
>
> Not easily, AFAIR.  But I don't think I understand the nature of the
> problem you are experiencing, see above.

The nature of the problem is the inability of ispell.el to maintain
multiple processes with different dictionaries simultaneously
that is required by flyspell-mode.

What is needed here is a hash of processes where the key is
the ispell program name and its dictionary arguments.

bug#69703: Ispell process restarted too often

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 69703@debbugs.gnu.org
> Date: Sun, 10 Mar 2024 19:10:19 +0200
> 
> > If the speller process is killed each time before starting a new one,
> > how come there's more than just one running at any given time?  IOW,
> > how come you have "hundreds of hunspell processes" in memory?  Does
> > killing a hunspell process fail for some reason?
> 
> Actually, hundreds were when I tried to use buffer-local ispell-process
> because then ispell-internal-change-dictionary didn't kill ispell-process
> but ispell-init-process created new processes.
> 
> But with the default configuration there are "only" a dozen of processes.

That's also unexpected.  There should be only one.

> I guess one process per buffer.  And this is even in case when ispell-process
> is NOT buffer-local.  I noticed that all these processes are created
> after loading the desktop buffers.

Please show a reproduction recipe, as I didn't know this is possible.

> But with the default configuration ispell-process is killed and started
> "only" on switching buffers that is very annoying since most of the time
> I see these messages in the echo area:
> 
>   Ispell process killed
>   Starting new Ispell process /usr/bin/hunspell ...
>   Ispell process killed
>   Starting new Ispell process /usr/bin/hunspell ...
>   Ispell process killed
>   Starting new Ispell process /usr/bin/hunspell ...

If the problem is with the messages, it can be easily solved by adding
some optional feature.  But I understand there are more urgent issues,
so let's discuss those first, okay?

> > Not easily, AFAIR.  But I don't think I understand the nature of the
> > problem you are experiencing, see above.
> 
> The nature of the problem is the inability of ispell.el to maintain
> multiple processes with different dictionaries simultaneously
> that is required by flyspell-mode.
> 
> What is needed here is a hash of processes where the key is
> the ispell program name and its dictionary arguments.

Feel free to submit patches for that, and thanks.  (But didn't you
just complain about one speller process per buffer?)

bug#69703: Ispell process restarted too often

[Juri Linkov]

>> I noticed that all these processes are created
>> after loading the desktop buffers.
>
> Please show a reproduction recipe, as I didn't know this is possible.

Here is what happens during loading the desktop:

1. When creating a new file buffer, 'ispell-kill-ispell' is called
   for every buffer because its personal dictionary is different
   from the previously restored buffer.

2. Then in the middle of 'ispell-kill-ispell',
   'project-mode-line-format' kicks in
   from 'redisplay_internal'.

3. 'project-mode-line-format' creates a temporary buffer
   and enables 'flyspell-mode' in it.

4. 'flyspell-mode' creates a new stray process.

5. 'ispell-kill-ispell' that was started at 1 above
   finishes killing the previous Ispell process.
   So the processes created by 'flyspell-mode' remain unhandled.

Ok, the bug is in 'project-mode-line-format'.
I will send a patch to fix it in a separate bug report.

>> But with the default configuration ispell-process is killed and started
>> "only" on switching buffers that is very annoying since most of the time
>> I see these messages in the echo area:
>> 
>>   Ispell process killed
>>   Starting new Ispell process /usr/bin/hunspell ...
>>   Ispell process killed
>>   Starting new Ispell process /usr/bin/hunspell ...
>>   Ispell process killed
>>   Starting new Ispell process /usr/bin/hunspell ...
>
> If the problem is with the messages, it can be easily solved by adding
> some optional feature.  But I understand there are more urgent issues,
> so let's discuss those first, okay?

Messages is not a problem.  Perpetual process restarting causes
noticeable performance degradation.

Since the bug will be fixed in 'project-mode-line-format',
the remaining problems are less urgent.

But still the remaining inconvenience is the following:

1. Loading the desktop takes more time since
it restarts the Ispell process for the every buffer:

Starting new Ispell process...
Ispell process killed
Starting new Ispell process...
Ispell process killed
Starting new Ispell process...
Ispell process killed
Starting new Ispell process...
Ispell process killed
Starting new Ispell process...

2. Ispell processes still restarted on editing another buffer.

>> > Not easily, AFAIR.  But I don't think I understand the nature of the
>> > problem you are experiencing, see above.
>> 
>> The nature of the problem is the inability of ispell.el to maintain
>> multiple processes with different dictionaries simultaneously
>> that is required by flyspell-mode.
>> 
>> What is needed here is a hash of processes where the key is
>> the ispell program name and its dictionary arguments.
>
> Feel free to submit patches for that, and thanks.  (But didn't you
> just complain about one speller process per buffer?)

Since implementing this is not urgent, here is a design outline
for possible future improvements:

1. Create a pool of Ispell processes.

2. Instead of killing Ispell processes
   just get the process from the pool
   by the ispell program name and its dictionary arguments.

3. For backward-compatibility in 'ispell-init-process'
   need to set the value of 'ispell-process' to the process
   extracted from the pool.

bug#69703: Ispell process restarted too often

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 69703@debbugs.gnu.org
> Date: Tue, 12 Mar 2024 09:45:14 +0200
> 
> >> The nature of the problem is the inability of ispell.el to maintain
> >> multiple processes with different dictionaries simultaneously
> >> that is required by flyspell-mode.
> >> 
> >> What is needed here is a hash of processes where the key is
> >> the ispell program name and its dictionary arguments.
> >
> > Feel free to submit patches for that, and thanks.  (But didn't you
> > just complain about one speller process per buffer?)
> 
> Since implementing this is not urgent, here is a design outline
> for possible future improvements:
> 
> 1. Create a pool of Ispell processes.
> 
> 2. Instead of killing Ispell processes
>    just get the process from the pool
>    by the ispell program name and its dictionary arguments.

This assumes that the number of different personal dictionaries is
much smaller than the number of buffers under flyspell-mode.  Since I
don't quite understand why you need so many personal dictionaries
(presumably for the same language?), I cannot say whether this
assumption is reasonable or scalable.  In general, if we want to
support buffer-local speller processes, we could have hundreds of
speller processes running on the system, which is not very nice, I
think.

The current design is generally based on the following assumptions:

  . a speller process is mostly needed in a single buffer at a time,
    the buffer where the user is typing
  . a speller process needs to be restarted only if you change the
    dictionary, which is done when you need to spell-check a different
    language
  . the number of languages for which the user needs spell-checking is
    small (or even spell-checked by a single speller, if that is
    Hunspell)

bug#69703: Ispell process restarted too often

[Juri Linkov]

close 69703 30.0.50
thanks

>> 1. Create a pool of Ispell processes.
>>
>> 2. Instead of killing Ispell processes
>>    just get the process from the pool
>>    by the ispell program name and its dictionary arguments.
>
> This assumes that the number of different personal dictionaries is
> much smaller than the number of buffers under flyspell-mode.  Since I
> don't quite understand why you need so many personal dictionaries
> (presumably for the same language?), I cannot say whether this
> assumption is reasonable or scalable.  In general, if we want to
> support buffer-local speller processes, we could have hundreds of
> speller processes running on the system, which is not very nice, I
> think.

I meant the case of per-project dictionaries with each project having
own .hunspell file, on the assumption that a set of identifiers in the
project-local dictionary is unique to each project.  This implies one
Isearch process for every project, but not for every buffer.

> The current design is generally based on the following assumptions:
>
>   . a speller process is mostly needed in a single buffer at a time,
>     the buffer where the user is typing
>   . a speller process needs to be restarted only if you change the
>     dictionary, which is done when you need to spell-check a different
>     language
>   . the number of languages for which the user needs spell-checking is
>     small (or even spell-checked by a single speller, if that is
>     Hunspell)

After fixing the project-mode-line-format bug, I see no more problems
when switching between projects with local dictionaries restarts
the Ispell process on typing, so I'm going to close this request.

bug#69703: Ispell process restarted too often

[Juri Linkov]

BTW, currently the order or enabling minor modes and
reading dir-local variables looks wrong: after visiting a file
first minor modes are enabled, then dir-local-variables
are set in the buffer.

So with (add-hook 'text-mode-hook 'flyspell-mode)
Ispell starts the Ispell process with the global dictionary.
Then later reads 'ispell-personal-dictionary' from .dir-locals.el
and restarts the Ispell process with another dictionary.

A workaround for configs is to force 'hack-dir-local-variables'
before enabling 'flyspell-mode' that is not a nice hack:

  (defun my-flyspell-mode ()
    ;; Avoid duplicate calls when ‘flyspell-mode’ is enabled by ‘.dir-locals.el’,
    ;; because the first call uses the global value of ‘ispell-personal-dictionary’,
    ;; and so restarts the Ispell process.
    (unless (progn (hack-dir-local-variables)
                   (member '(mode . flyspell) file-local-variables-alist))
      (flyspell-mode)))

  (add-hook 'text-mode-hook 'my-flyspell-mode)