machty / ember-concurrency

ember-concurrency is an Ember Addon that enables you to write concise, worry-free, cancelable, restartable, asynchronous tasks.
http://ember-concurrency.com
MIT License
690 stars 157 forks source link

RFC: add limiter task modifier #55

Open kellyselden opened 8 years ago

kellyselden commented 8 years ago

Basically this. I'm currently using it to send x number of requests per y milliseconds. It works but since it uses setTimeout, it doesn't block the ember test helpers for waiting (https://github.com/emberjs/ember.js/issues/3008). I wonder if it was natively in ember-concurrency, we could get this both testable the benefits of your canceling?

machty commented 8 years ago

How are you integrating this with ember-concurrency? Just having it do someTask.perform() on every rate-limited callback?

Any reason you can't just do:

rateLimitedFoo: task(function * () {
  this.get('foo').perform();
  yield timeout(RATE_LIMIT_PERIOD);
}).enqueue().maxConcurrency(N),

foo: task(function * () {
  // ...
}),
machty commented 8 years ago

Hmm I guess using maxConcurrency isn't exactly the same thing since in a rate-limited situation with, say, a threshold of 30 in a period of 5 minutes, since in that situation it'd be perfectly fine to fire off 30 tasks in the first millisecond but just defer anything after that until the 5 min mark, whereas maxConcurrency would only limit the number of concurrent tasks running, regardless of period.

kellyselden commented 8 years ago

Your example is very close. Some thoughts: if only one task was requested out of 30 in the pool, the timeout method above would block tests for that 5 mins, whereas a "native" solution could run the task and not block, then when the 31st task came in, to do a calculation for when to schedule it. Hope I make sense.

machty commented 8 years ago

I'm not sure a rate-limiter is something I'd implement as a first class feature but something I've been putting on the backburner is a public API for writing task modifiers, and perhaps this would be a good use case to start giving that some real consideration. So ideally with this nice public API, anyone could write e-c plugins that decide when/how tasks are scheduled/canceled in response to new task instances being .perform()ed, pre-existing instances running to completion, timer events, etc., including something like a rate limiter. I'd like to give this some more thought though; part of me wonders if there isn't a way to implement this cleanly with ember-concurrency, or if the real solution is to use a rate-limiting library that lets you configure which "setTimeout" fn gets used.

kellyselden commented 8 years ago

Good thoughts. I tried the window.setTimeout = Ember.run.later; in the beginning but it blew up in my face.

machty commented 8 years ago

If you're curious, the place where .enqueue() .drop() and .restartable() are implemented is here: https://github.com/machty/ember-concurrency/blob/master/addon/-buffer-policy.js