threads.js icon indicating copy to clipboard operation
threads.js copied to clipboard

Feat/dynamic pools

Open haggholm opened this issue 5 years ago • 3 comments

Dynamically sized pools: If an idle interval is specified, periodically kill idle worker threads (optionally, specify a minimum size below which the pool size may not shrink)

haggholm avatar Apr 21 '20 00:04 haggholm

Interesting feature. Thanks for sharing, @haggholm!

It's a pretty advanced feature that would now increase the already complex pool implementation quite a bit. May I suggest a slightly different implementation approach?

It would be great to implement the auto-resizing as an additional "layer" on top of the generic pool implementation. How about we only add a resize() to the generic pool and then have a separate AutoResizingPool or so that uses the generic pool and only contains the resizing behavior logic.

Additional benefit: The auto-resizing code can then be tree-shaken away if not used (which will be the case for most pool users). You could also turn the auto-resizing pool into a separate package then if you want to.

andywer avatar Apr 21 '20 18:04 andywer

It would be great to implement the auto-resizing as an additional "layer" on top of the generic pool implementation. How about we only add a resize() to the generic pool and then have a separate AutoResizingPool or so that uses the generic pool and only contains the resizing behavior logic.

I’m pretty sure that’s in general a very good idea. This week/month in particular, well—I added this patch to your package because unexpected corner cases in a more generic pool package, combined with threads for basic workers, bit me in the ass (in this case generic-pool and the surprising fact that borrowed + available !== size!). So in part, what I was trying to achieve here was dynamic pool size without trying to be so generic as to be potentially surprising, because while being generic has tremendous benefits, premature generic-ness is almost as bad as Dijsktra told us premature optimization is…

Well, I’m not trying to be disagreeable; I love this package, I'm very happy with how receptive you are to contributions, and your general point is entirely sound; but (from my POV) it needs to not be desperately/excessively generic, and I’d like to keep a nice, simple API like this, so I may not have the resources to refactor immediately.

haggholm avatar Apr 24 '20 01:04 haggholm

Hey @haggholm! Any chance of an overhaul of the PR in the near future?

Otherwise I will close it for now.

andywer avatar Aug 10 '20 15:08 andywer