Open Yuras opened 7 months ago
I wonder how this could be fixed.
I guess mkDebounce
could also provide a finalizer?
Or create a function that captures its own MVar and doesn't need to fork a thread?
mkDebounce :: IO () -> IO (IO ())
mkDebounce action = do
baton <- newMVar ()
pure $ do
mBaton <- tryTakeMVar baton
case mBaton of
Nothing -> pure ()
Just () -> do
-- handle Leading/Trailing
-- and `putMVar ()` when done with action + delay
@kazu-yamamoto would this idea be an ok replacement for Control.Debounce.mkDebounce
? Or do you see any issues with it? (a finally putMVar
might also be needed, come to think of it)
This module was created by @snoyberg. I don't have strong opinions but yes, cleaning up threads is a MUST.
Will try to reproduce with my PR #996 at the earliest convenience.
Note that the recent GHC has GHC.Conc.Sync.listThreads
.
Using it with lableThreads
, we can check if the clean up works well.
Now "label" can be passwd to auto-update stuffs via fooThreadName
.
I'll add listThreads
to my reproduction.
I don't know what you mean with "passing 'label' to 'auto-update stuffs'", though :thinking:
See updateThreadName.
There is a high change that the issue is actually caused by labelThread
, see https://gitlab.haskell.org/ghc/ghc/-/issues/23949
The reproducer in the issue description is probably wrong, it's not letting the threads chance to receive BlockedIndefinitelyOnMVar
. There should be threadDelay
and performMajorGC
somewhere in the loop.
There is a high change that the issue is actually caused by
labelThread
, see https://gitlab.haskell.org/ghc/ghc/-/issues/23949The reproducer in the issue description is probably wrong, it's not letting the threads chance to receive
BlockedIndefinitelyOnMVar
. There should bethreadDelay
andperformMajorGC
somewhere in the loop.
labelThread
was not added in the debouncing function(s) until auto-update-0.2.2
, which was released about a month ago, so unless the labelThread
issue was always a problem for literally every fork
, even if the labelThread
wasn't called. This would still be an issue.
I've ran OP's example on master
with GHC 9.6.6
and GHC 9.2.8
and found no significant difference.
(They were both always above 100MB total memory in use, and would also sometimes result in the 130~ish MB the OP showed.)
And I've ran it with my PR #996 on GHC 9.6.6, which used a lot less memory, and also found that when I added a print statement in the debounce action, the printing would stagger and lag with the current code, but with my PR it was a lot smoother and just flooded my screen as fast as possible, instead of having hiccups and freezes.
Now some tests with listThreads
.
The master
branch kept about 57000 threads active if the debounceAction
was only pure ()
. It kept 47000 active if the debounceAction
had a threadDelay 1_000_000
and this goes down the more the threadDelay
increases... not entirely sure why. (See Addendum 1
)
The #996 way of doing debouncing still has 11000~ish threads open after all that, and this goes down by a bit with a 1s threadDelay
as the debounceAction
, but somehow the amount of threads open at the end increases again if the debounceAction
is set to 2s threadDelay
? :thinking: (See Addendum 2
)
This might have to do with the GC taking longer to recognize the threads can be cleaned up? Or something like that? I honestly don't have too much knowledge about the GHC runtime to know how it cleans up green threads, and how accurate listThreads
is.
But all in all, #996 does seem like an improvement :shrug:
Ah, after adding some BangPatterns
in front of the results from listThread
(like !nowThreads <-
), this is the result with #996 (debounceAction
is pure ()
):
And this is the result on the master
thread:
So it does seem the new way of debouncing actually cleans up the threads, whereas the current one leaks at least somewhat?
1 sec leaves less:
2 sec leaves even less:
But 4 sec leaves more?:
mkDebounce
internally forks a thread, but never kills it. To reproduce, run the following with+RTS -s -RTS
and observe memory usage.Output:
132MiB memory is too much for the above program.
As a possible solution, consider attaching a finalizer to the debounced action that will kill the thread. I'd be happy to prepare a patch if you agree with such a solution. Other possibility is to use a closable channel instead of
MVar
. I didn't try it though.A bit of context: fast-logger uses
mkDebounce
to flush logs, so each logger comes with a thread which never exits.Thanks!