search

STRML / node-toobusy

Don't let your Node.JS server fall over when it's too busy.
366 stars 36 forks source link

Don't call `start()` when importing module #26

Closed robross0606 closed 2 months ago

robross0606 commented 2 months ago

It is a bit of a code smell to invoke state() the second someone imports the module. It makes the library lest testable and controllable. Allow users to import the module and then call start() and shutdown() as they desire.

robross0606 commented 2 months ago

start() is also called every time interval() is called, which means you're creating an interval, probably killing it, and creating a new one just because one is created when the module is imported.

robross0606 commented 2 months ago

Also, why is interval capped at 16ms?

rickfero commented 2 months ago

Also, why is interval capped at 16ms?

I'm pretty sure this project is dead and incredibly out of date. Don't use it.

robross0606 commented 2 months ago

@rickfero, thank you for the candid advice. I was attracted to the smoothing algorithms in this project. Short of using perf_hooks which has its own issues, do you know of a good replacement?

rickfero commented 2 months ago

No sorry, I don't think there are any working libraries now. The various too-busy libraries from 10 years ago never worked well including this one.

robross0606 commented 2 months ago

What about ones not based on too-busy at all? Something completely different?

rickfero commented 2 months ago

Sorry, I don't know. ask https://www.reddit.com/r/node/

robross0606 commented 2 months ago

Okay, thanks for the info.

STRML commented 2 months ago

I'd ask what your needs are. Interval is not allowed to be less than 16ms because (at the time) it was generally not possible to get reliable results that quickly.

I agree that start() should not be invoked at import. Changing it would be breaking, of course. But you could work around this by immediately calling shutdown(), then interval().