Use microtask queue to flush autoruns... #306

rwjblue · 2018-01-17T05:01:51Z

Replace the private _platform option, with a new _buildPlatform
Add interfaces for the constructor args for the Backburner class.
Move the main implementation of Backburner.prototype.end into a
private method (_end) that is aware of if the end is from an autorun
or manual run...
Extract steps to schedule an autorun out into stand alone private
method (_scheduleAutorun)
Leverage microtask queue (via either promise.then(...) or
MutationObserver) to schedule an autorun
Leverage microtask queue to advance from one queue to the next

This PR is just a small stepping stone towards leveraging microtasks a bit more (with the goal of removing Ember's autorun assertion)...

rwjblue · 2018-01-18T17:21:10Z

With the last commit (using MutationObserver when possible), the new test (confirming autorun flush happens before a bb.later(fn, 0)) passes on all supported browsers.

stefanpenner · 2018-01-18T19:56:40Z

seems good, this may need to be major bump, so it can ride the canary train in ember. Some caveats may exist re: popup blockers and window.open being run in autoruns.

Table of funky behavior is here: #181 (we may deem it irrelevant)

rwjblue · 2018-01-18T21:54:19Z

seems good, this may need to be major bump, so it can ride the canary train in ember

Ya, we definitely want it to go through a full canary -> beta -> release cycle so we can ensure this has no negative ramifications.

FWIW, I have been working on the next step (advancing from queue to queue during an autorun via microtask queue flushes) in rwjblue#1 if you are interested in the followup progress...

stefanpenner · 2018-01-18T23:43:38Z

FWIW, I have been working on the next step (advancing from queue to queue during an autorun via microtask queue flushes) in rwjblue#1 if you are interested in the followup progress...

Sounds like a good approach.

My long-term thought was to essentially make auto-run default, and if Ember.run is wrapping, abide by it. We can do a VC call if you want someone to chat about it.

rwjblue · 2018-01-19T00:49:26Z

Yep, that’s exactly the direction I am pushing on. Will pester you tomorrow to chat through it some more 😆...

stefanpenner · 2018-01-19T20:04:50Z

lib/index.ts

    platform.clearNext = _platform.clearNext || platform.clearTimeout;
    platform.now = _platform.now || (() => Date.now());

-    this._platform = platform;
+    // TODO: do we need this?
+    if (_platform.next) {


should we make this configurable, and just plug in RSVP's asap in ember? Then we avoid duplicate code

We could make asap also use native Promise for its asap.

@stefanpenner - I think we want to avoid the extra level of queuing that RSVP.asap does. Specifically we need to leverage the fact that microtasks that we enqueue are in the correct position (e.g. interleaving of native promises and RSVP.Promises should maintain the same resolve ordering).

stefanpenner · 2018-01-19T23:50:44Z

lib/backburner/host-platform.ts

@@ -0,0 +1,53 @@
+export interface IPlatform {
+  setTimeout(fn: () => void, ms: number): any;


fn: Function

stefanpenner · 2018-01-19T23:50:53Z

lib/backburner/host-platform.ts

+const SET_TIMEOUT = setTimeout;
+const NOOP = () => {};
+
+export function buildDefaultPlatform(flush: () => void): IPlatform {


should we just configure asap?

I’m concerned with swapping to asap as it adds another layer of queuing and the exact ordering of interleaved RSVP promises, native promises, and BB queue transitions are super important...

However, this “host” interface style was explicitly designed to allow Ember to use RSVP.asap if it wants (it only needs to pass a _buildPlatform function that uses RSVP.asap for the provided next (you can see some examples in the tests)...

stefanpenner · 2018-01-19T23:56:26Z

lib/index.ts

@@ -137,7 +151,7 @@ export default class Backburner {
    return current;
  }

-  public end() {
+  public end(fromAutorun = false) {


I would prefer to not expose this flag via a public API, we should maybe create _end that can have w/e flag, but if at all possible keep the existing one pristine (if possible).

Yep, totally makes sense.

* Replace the private `_platform` option, with a new `_buildPlatform` * Add interfaces for the constructor args for the `Backburner` class. * Move the main implementation of `Backburner.prototype.end` into a private method (`_end`) that is aware of if the end is from an autorun or manual run... * Extract steps to schedule an autorun out into stand alone private method (`_scheduleAutorun`) * Leverage microtask queue (via either `promise.then(...)` or `MutationObserver`) to schedule an autorun * Leverage microtask queue to advance from one queue to the next

rwjblue · 2018-02-01T05:05:23Z

I just ran a series of benchmarks comparing various versions of backburner (v0.3.1 used by Ember 2.13, v1.3.4 used by Ember 2.18, v2.1.0 used by Ember 3.0.0-beta.5, master at e65437d, and this PR's changes) against a complex real world app.

As you can see below, compared to e65437d we have a very high confidence that this has a positive impact on performance (estimated difference is -153ms):

Full results

rwjblue · 2018-02-01T05:09:05Z

@stefanpenner / @krisselden - I'd like to land this (given the strong evidence shown above) so that we can update Ember and continue fleshing out our plans RE: native promises and whatnot. Any objections?

Addressed and replied inline...

rwjblue requested a review from krisselden January 17, 2018 05:01

rwjblue force-pushed the autorun-microtask branch 3 times, most recently from 63ecfbf to 9e04bb8 Compare January 18, 2018 17:11

rwjblue changed the title ~~Use Promise's to run scheduled autorun's in microtask queue.~~ Use microtask queue to flush autoruns... Jan 18, 2018

rwjblue force-pushed the autorun-microtask branch from 9e04bb8 to c7885f2 Compare January 18, 2018 17:28

stefanpenner mentioned this pull request Jan 18, 2018

Experiments #180

Closed

5 tasks

stefanpenner reviewed Jan 19, 2018

View reviewed changes

rwjblue force-pushed the autorun-microtask branch 3 times, most recently from e3286b5 to 33755a5 Compare January 19, 2018 20:52

stefanpenner force-pushed the autorun-microtask branch from 33755a5 to c2965dc Compare January 19, 2018 23:50

stefanpenner previously requested changes Jan 19, 2018

View reviewed changes

rwjblue force-pushed the autorun-microtask branch from 84a04d7 to 984ebcd Compare January 20, 2018 21:12

bekzod mentioned this pull request Jan 21, 2018

move platform methods extracting from constructor #313

Closed

This was referenced Jan 21, 2018

remove deprecated deferOnce, defer and setTimeout #290

Open

Moving to micro-task for stuff #181

Open

rwjblue force-pushed the autorun-microtask branch from 984ebcd to 0fa00ae Compare January 31, 2018 19:29

rwjblue force-pushed the autorun-microtask branch from 0fa00ae to 27690de Compare February 1, 2018 04:47

rwjblue requested a review from stefanpenner February 1, 2018 05:09

stefanpenner approved these changes Feb 2, 2018

View reviewed changes

rwjblue merged commit 855b208 into BackburnerJS:master Feb 2, 2018

rwjblue deleted the autorun-microtask branch February 2, 2018 23:37

sivakumar-kailasam mentioned this pull request Oct 11, 2018

removes extra info for Ember.run wrapping ember-learn/guides-source#192

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use microtask queue to flush autoruns... #306

Use microtask queue to flush autoruns... #306

rwjblue commented Jan 17, 2018 •

edited

Loading

rwjblue commented Jan 18, 2018 •

edited

Loading

stefanpenner commented Jan 18, 2018 •

edited

Loading

rwjblue commented Jan 18, 2018

stefanpenner commented Jan 18, 2018

rwjblue commented Jan 19, 2018

stefanpenner Jan 19, 2018

stefanpenner Jan 19, 2018

rwjblue Jan 19, 2018 •

edited

Loading

stefanpenner Jan 19, 2018

stefanpenner Jan 19, 2018

rwjblue Jan 20, 2018

stefanpenner Jan 19, 2018

rwjblue Jan 20, 2018

rwjblue commented Feb 1, 2018

rwjblue commented Feb 1, 2018

		@@ -0,0 +1,53 @@
		export interface IPlatform {
		setTimeout(fn: () => void, ms: number): any;

Use microtask queue to flush autoruns... #306

Use microtask queue to flush autoruns... #306

Conversation

rwjblue commented Jan 17, 2018 • edited Loading

rwjblue commented Jan 18, 2018 • edited Loading

stefanpenner commented Jan 18, 2018 • edited Loading

rwjblue commented Jan 18, 2018

stefanpenner commented Jan 18, 2018

rwjblue commented Jan 19, 2018

stefanpenner Jan 19, 2018

Choose a reason for hiding this comment

stefanpenner Jan 19, 2018

Choose a reason for hiding this comment

rwjblue Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

stefanpenner Jan 19, 2018

Choose a reason for hiding this comment

stefanpenner Jan 19, 2018

Choose a reason for hiding this comment

rwjblue Jan 20, 2018

Choose a reason for hiding this comment

stefanpenner Jan 19, 2018

Choose a reason for hiding this comment

rwjblue Jan 20, 2018

Choose a reason for hiding this comment

rwjblue commented Feb 1, 2018

rwjblue commented Feb 1, 2018

rwjblue commented Jan 17, 2018 •

edited

Loading

rwjblue commented Jan 18, 2018 •

edited

Loading

stefanpenner commented Jan 18, 2018 •

edited

Loading

rwjblue Jan 19, 2018 •

edited

Loading