Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PERF] Ajax should join an existing run if one exists #3943

Merged
merged 1 commit into from
Nov 20, 2015

Conversation

chadhietala
Copy link
Contributor

@chadhietala chadhietala commented Nov 19, 2015

Prior to this commit we would always create a new runloop, which would lead to nested runloops. Instead this will join a runloop if one is already running or create one if one does not exist.

@fivetanley
Copy link
Member

So Ember has its own instance of Backburner.... I'm not sure what we should be using here. This is probably right, but I don't know enough about why we have our own instance.

cc @igorT @stefanpenner

@runspired
Copy link
Contributor

There's a separate instance? ime ED has it's own queues, but those are added to the same instance. Or at least they were when I poked around a while back, probably around beta 18.

@fivetanley
Copy link
Member

@krisselden
Copy link
Contributor

@fivetanley this comes into play if you have cached or server side rendered response you fullfill the ajax with and already have a run loop.

@krisselden
Copy link
Contributor

@chadhietala can you flatten?

@chadhietala
Copy link
Contributor Author

@krisselden squashed.

@stefanpenner
Copy link
Member

LGTM.

For those reading along ofcourse for most people this doesn't make a difference, But if they happen to batch multiple XHR at a lower layer then this, they can easily now wrap that base in a run, batching these higher level one.

@stefanpenner
Copy link
Member

@fivetanley not to worry, that should not matter, the ED bb should stay the way it is. (at least for now)

@runspired it has its own bb instance, and the queues are not shared between instances. the aim was avoid plan interference of two different interleaving priority queues.

The ember bb queue, can run a task which triggers the ED queue to grow. At which time, the ED queue must flush entirely. This ensures the various multi-phase ED relationship synching steps can reach a consistent state before releasing back to ambient flush.

In theory with some changes to BB this could have been supported with 1 BB instance, but the current approach required no extra BB work, keeping the complexity down, and was semantically the same.

bmac added a commit that referenced this pull request Nov 20, 2015
[PERF] Ajax should join an existing run if one exists
@bmac bmac merged commit 15bb4b2 into emberjs:master Nov 20, 2015
@bmac
Copy link
Member

bmac commented Nov 20, 2015

Thanks @chadhietala.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants