search

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
689 stars 157 forks source link

`env.async` uses `run.once` but passes a new function #437

Closed stefanpenner closed 3 years ago

stefanpenner commented 3 years ago

Both usages of this.env.async appear to rely on run.once, but pass dynamically created functions on each occurrence. Which means the de-duplication effort done by run.once is wasted:

file: addon/-private/external/task-instance/executor.js has: 103: this.env.async(() => this.proceedSync(yieldResumeType, value)); 344: this.env.async(() => {

where async is implemented via join(() => once(null, callback)) https://github.com/machty/ember-concurrency/blob/f53656876748973cf6638f14aab8a5c0776f5bba/addon/-private/ember-environment.js#L13

The most conservative change to correct ^ would like be: join(() => schedule('actions', callback))

maxfierke commented 3 years ago

oh snap, good find! We should fix that!

Out of curiosity, is there a measurable performance impact you're seeing from this?

stefanpenner commented 3 years ago

Out of curiosity, is there a measurable performance impact you're seeing from this?

Nah, it's most likely a tiny paper-cut among many :P. It isn't super critical, but something I noticed while debugging some code using ember-concurrency and I figured I would share :)