Removes store._adapterRun

[runspired]

With #4698 having landed, it's potentially possible we can remove some if not all calls to store._adapterRun. This PR currently removes all calls, leading to a small handful of failed tests which I'm investigating. I'll also run it against LI's test suite once those pass (if I can make them pass) to see if there are undocumented behaviors that this might break.

Benefits

• many fewer closures
• reduced overhead (one layer of abstraction fewer)
• easier to understand the data flow in finders
• simplifies the "async story" in ember-data, helping us find more avenues for improving our async data flow story in the future.
• gets us within a very very short step of removing backburner entirely. Only necessary thing would be for a true microtask based flush mechanism to be available.

cc @igor for recommending I investigate this.

[runspired]

I believe this is ready to merge, but per @igorT's request I'm going to get this running against our app's test suite to see if there are any unknown consequences.

[stefanpenner]

To be clear this may not be what backburner itself considers a nested run-loop, as it has its own concept of this. Internal BB handles nested run-loops via a stack of run-loops

For example:

Ember.run(() => Ember.run(() => /* nested run loop */));

Both have the same queues, and an inner one may capture things actually intended for the outer one (today). Which will have nested renders etc.

---

Ember.run(() => store._backburner.run(() => /* nested but entirely independently run-loop */));

Two run loops, but different queues so Ember.run.schedules within the bb.run will escape to the outer one, but bb.schedules wil remain captured.

---

TL;DR words are hard, maybe we should re-jigger the words in this comment.

[runspired]

@stefanpenner this is actually a nested run loop for EDs internal backburner.

[runspired]

The outer loop is opened by _push.

[stefanpenner]

@runspired I don't believe so, the comment is within a promise.then which fulfills on the embers bb, escaping the store's bb.

[runspired]

@stefanpenner I'll reason through this a little bit later but there's definitely something ED bb related going on here.

[stefanpenner]

@stefanpenner I'll reason through this a little bit later but there's definitely something ED bb related going on here.

I can believe that, but it may require some additional investigation. Let me know if you find something else. Or if i can provide clarity to my above claims.

[runspired]

@stefanpenner so after doing a little logging and reasoning through, that run is needed for two reasons but could have been (and now is) a join.

TL;DR

• it is usually (but not always) the case that even though we are inside a Promise.resolve().then() callback we are still within an active ED backburner run-loop. We need this for the case in which we are not in an active run loop.
• this code calls _push, which joins or creates a run-loop, however, store.didSaveRecord is outside of that _push and should also be a part of that same loop. By joining here we ensure that both the inner and the outer call to store._push utilize the same loop along with store.didSaveRecord.

[stefanpenner]

callback we are still within an active ED backburner run-loop.

I don't believe this is possible, example?

---

The only way I can think of getting into this, should be a high priority fix itself:

ed._run(function() {
  Ember.run(function() {
    RSVP.Promise.resolve().then(function() {
     ed._run.join(function() {

    });
  });
});

The other way, (a variation of the above) is if the ed run auto-runs, which also should be addressed.

[runspired]

The only way I can think of getting into this, should be a high priority fix itself:

This is actually precisely what is happening, however I don't believe we have a path off of this at the moment.

[stefanpenner]

@runspired whats the easiest way for me to replicate. I would like to see if we can confirm, and get off.

[runspired]

@stefanpenner set a flag inside of store._push, turn it off at the end of store._push, throw an error or log something in _commit only if that flag is on once inside of the Promise.resolve().then() callback.

[runspired]

places where ember-data calls into Ember's run loop directly and could be contributing:

[runspired]

Actually this seems the most likely culprit:

[runspired]

I wonder if run() calls in tests perhaps lead to this being a different interlacing than it would be in the real world.

[runspired]

This may also be affected by everything chaining off of resolver.resolve

[stefanpenner]

@runspired are you in the office tomorrow? We should just pair.