yoshuawuyts / barracks

:mountain_railway: action dispatcher for unidirectional data flows
MIT License
177 stars 22 forks source link

shared setTimeout() #74

Open yoshuawuyts opened 8 years ago

yoshuawuyts commented 8 years ago

currently each send() call runs on its own setTimeout() call which causes a fair bit of lag - we should pool them similar to how process.nextTick() works - this'll save us a whole lotta delay on average, probably

davidguttman commented 8 years ago

Something like this? https://www.nczonline.net/blog/2009/08/11/timed-array-processing-in-javascript/

Basically, call as many as you can in 50ms, then take a break before doing the next bunch.

anandthakker commented 8 years ago

@yoshuawuyts As I found in a recent project using barracks, this problem can actually get quite a bit worse if there are lots of other scripts on the page (we were building components that were being embedded on pages w/ a lot of stuff already going on).

My solution was to--very carefully--fork barracks and remove the setTimeout entirely. That's probably not what you want for barracks itself, but I think that using next-tick instead of setTimeout might be a good solution, because it uses MutationObserver when supported to ensure callbacks are resolved as microtasks at the end of the current script/timer execution, rather than being queued behind other events or tasks.

One caveat: beware that by default, next-tick will use browserify's implementation of process.nextTick instead of its own, so a slight hack (or a change to node-process) would be required in order to actually get the MutationObserver implementation to be used.

yoshuawuyts commented 8 years ago

Oh, that's actually a really good suggestion - I keep hearing about this microtask thing and it sounds heaps good; will have to read up on it. Glad barracks worked for you on your last project by the way; ended up looking heaps cool! :D

Would something like next-tick work for the use case you had on your project?

On Mon, Nov 14, 2016 at 6:33 PM Anand Thakker notifications@github.com wrote:

@yoshuawuyts https://github.com/yoshuawuyts As I found in a recent project using barracks, this problem can actually get quite a bit worse if there are lots of other scripts on the page (we were building components that were being embedded on pages w/ a lot of stuff already going on).

My solution was to--very carefully--fork barracks and remove the setTimeout entirely. That's probably not what you want for barracks itself, but I think that using next-tick https://github.com/medikoo/next-tick instead of setTimeout might be a good solution, because it uses MutationObserver when supported to ensure callbacks are resolved as microtasks at the end of the current script/timer execution, rather than being queued behind other events or tasks.

One caveat: beware that by default, next-tick will use browserify's implementation of process.nextTick https://github.com/medikoo/next-tick/issues/9 instead of its own, so a slight hack (or a change to node-process https://github.com/defunctzombie/node-process) would be required in order to actually get the MutationObserver implementation to be used.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/yoshuawuyts/barracks/issues/74#issuecomment-260386041, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlegrHw1nzz18oFz_9kuVckljKcs0Iks5q-I1dgaJpZM4KhSaM .

anandthakker commented 8 years ago

@yoshuawuyts Yeah, I think next-tick essentially would have worked--in fact, I did use it in a couple of other parts of the project.

In our specific case, I opted not to simply change barracks's setTimeout() to nextTick(). Instead, I hacked send to make it dispatch synchronously by default, because that was the quickest way for me to make sure certain remote requests were fired off as soon as possible... BUT, the only reason I did it that way was because it was very last-minute and I didn't have time to do much refactoring--this was really just a last minute hack.

yoshuawuyts commented 8 years ago

Cool, thanks for sharing! Feeling confident having a nexttick solution will be the right move :D

On Wed, Nov 16, 2016, 17:47 Anand Thakker notifications@github.com wrote:

@yoshuawuyts https://github.com/yoshuawuyts Yeah, I think next-tick essentially would have worked--in fact, I did use it in a couple of other parts of the project.

In our specific case, I opted not to simply change barracks's setTimeout() to nextTick(). Instead, I hacked send to make it dispatch synchronously by default, because that was the quickest way for me to make sure certain remote requests were fired off as soon as possible... BUT, the only reason I did it that way was because it was very last-minute and I didn't have time to do much refactoring--this was really just a last minute hack.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/yoshuawuyts/barracks/issues/74#issuecomment-261000901, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlescBQvHnRxhVvKCtyjVN5gsSLCq2ks5q-zOCgaJpZM4KhSaM .