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] use micro-queue #4668

Merged
merged 1 commit into from
Dec 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions addon/-private/system/relationships/state/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ function shouldFindInverse(relationshipMeta) {
function createRelationshipFor(internalModel, relationshipMeta, store) {
let inverseKey;
let inverse = null;

if (shouldFindInverse(relationshipMeta)) {
inverse = internalModel.type.inverseFor(relationshipMeta.key, store);
}
Expand Down
101 changes: 68 additions & 33 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,24 @@ Store = Service.extend({
this.recordArrayManager = RecordArrayManager.create({
store: this
});
this._pendingSave = [];
this._instanceCache = new ContainerInstanceCache(getOwner(this), this);

//Used to keep track of all the find requests that need to be coalesced
/*
Ember Data uses several specialized micro-queues for organizing
and coalescing similar async work.

These queues are currently controlled by a flush scheduled into
ember-data's custom backburner instance.
*/
// used for coalescing record save requests
this._pendingSave = [];
// used for coalescing relationship setup needs
this._pushedInternalModels = [];
// stores a reference to the flush for relationship setup
this._relationshipFlush = null;
// used to keep track of all the find requests that need to be coalesced
this._pendingFetch = MapWithDefault.create({ defaultValue() { return []; } });

this._instanceCache = new ContainerInstanceCache(getOwner(this), this);
},

/**
Expand Down Expand Up @@ -1850,25 +1863,27 @@ Store = Service.extend({
let pending = this._pendingSave.slice();
this._pendingSave = [];

pending.forEach((pendingItem) => {
for (let i = 0, j = pending.length; i < j; i++) {
let pendingItem = pending[i];
let snapshot = pendingItem.snapshot;
let resolver = pendingItem.resolver;
let record = snapshot._internalModel;
let adapter = this.adapterFor(record.modelClass.modelName);
let internalModel = snapshot._internalModel;
let adapter = this.adapterFor(internalModel.modelClass.modelName);
let operation;

if (get(record, 'currentState.stateName') === 'root.deleted.saved') {
if (get(internalModel, 'currentState.stateName') === 'root.deleted.saved') {
return resolver.resolve();
} else if (record.isNew()) {
} else if (internalModel.isNew()) {
operation = 'createRecord';
} else if (record.isDeleted()) {
} else if (internalModel.isDeleted()) {
operation = 'deleteRecord';
} else {
operation = 'updateRecord';
}

resolver.resolve(_commit(adapter, this, operation, snapshot));
});
}

},

/**
Expand All @@ -1891,8 +1906,8 @@ Store = Service.extend({
}
if (data) {
// normalize relationship IDs into records
this._backburner.schedule('normalizeRelationships', this, this._setupRelationships, internalModel, data);
this.updateId(internalModel, data);
this._setupRelationshipsForModel(internalModel, data);
} else {
assert(`Your ${internalModel.type.modelName} record was saved to the server, but the response does not have an id and no id has been set client side. Records must have ids. Please update the server response to provide an id in the response or generate the id on the client side either before saving the record or while normalizing the response.`, internalModel.id);
}
Expand Down Expand Up @@ -2007,6 +2022,7 @@ Store = Service.extend({
heimdall.increment(_load);
let internalModel = this._internalModelForId(data.type, data.id);

// TODO @runspired move this out of here
internalModel.setupData(data);

this.recordArrayManager.recordDidChange(internalModel);
Expand Down Expand Up @@ -2266,17 +2282,18 @@ Store = Service.extend({
},

/*
Push some data into the store, without creating materialized records.
Push some data in the form of a json-api document into the store,
without creating materialized records.

@method _push
@private
@param {Object} data
@param {Object} jsonApiDoc
@return {DS.InternalModel|Array<DS.InternalModel>} pushed InternalModel(s)
*/
_push(data) {
_push(jsonApiDoc) {
let token = heimdall.start('store._push');
let internalModelOrModels = this._backburner.join(() => {
let included = data.included;
let included = jsonApiDoc.included;
let i, length;

if (included) {
Expand All @@ -2285,23 +2302,23 @@ Store = Service.extend({
}
}

if (Array.isArray(data.data)) {
length = data.data.length;
if (Array.isArray(jsonApiDoc.data)) {
length = jsonApiDoc.data.length;
let internalModels = new Array(length);

for (i = 0; i < length; i++) {
internalModels[i] = this._pushInternalModel(data.data[i]);
internalModels[i] = this._pushInternalModel(jsonApiDoc.data[i]);
}
return internalModels;
}

if (data.data === null) {
if (jsonApiDoc.data === null) {
return null;
}

assert(`Expected an object in the 'data' property in a call to 'push' for ${data.type}, but was ${typeOf(data.data)}`, typeOf(data.data) === 'object');
assert(`Expected an object in the 'data' property in a call to 'push' for ${jsonApiDoc.type}, but was ${typeOf(jsonApiDoc.data)}`, typeOf(jsonApiDoc.data) === 'object');

return this._pushInternalModel(data.data);
return this._pushInternalModel(jsonApiDoc.data);
});
heimdall.stop(token);
return internalModelOrModels;
Expand Down Expand Up @@ -2343,17 +2360,32 @@ Store = Service.extend({
// Actually load the record into the store.
let internalModel = this._load(data);

this._backburner.schedule('normalizeRelationships', this, this._setupRelationships, internalModel, data);
this._setupRelationshipsForModel(internalModel, data);

return internalModel;
},

_setupRelationships(record, data) {
_setupRelationshipsForModel(internalModel, data) {
this._pushedInternalModels.push(internalModel, data);
if (this._relationshipFlush === null) {
this._relationshipFlush = this._backburner.schedule('normalizeRelationships', this, this._setupRelationships);
}
},

_setupRelationships() {
heimdall.increment(_setupRelationships);
// This will convert relationships specified as IDs into DS.Model instances
// (possibly unloaded) and also create the data structures used to track
// relationships.
setupRelationships(this, record, data);
let pushed = this._pushedInternalModels;
this._pushedInternalModels = [];
this._relationshipFlush = null;

for (let i = 0, l = pushed.length; i < l; i += 2) {
// This will convert relationships specified as IDs into DS.Model instances
// (possibly unloaded) and also create the data structures used to track
// relationships.
let internalModel = pushed[i];
let data = pushed[i + 1];
setupRelationships(this, internalModel, data);
}
},

/**
Expand Down Expand Up @@ -2612,6 +2644,9 @@ Store = Service.extend({

willDestroy() {
this._super(...arguments);
this._pushedInternalModels = null;
this._backburner.cancel(this._relationshipFlush);
this._relationshipFlush = null;
this.recordArrayManager.destroy();
this._instanceCache.destroy();

Expand All @@ -2623,7 +2658,7 @@ Store = Service.extend({
return;
}

assert(`A ${relationship.parentType} record was pushed into the store with the value of ${relationship.key} being ${inspect(resourceIdentifier)}, but ${relationship.key} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(resourceIdentifier));
assert(`A ${relationship.record.modelName} record was pushed into the store with the value of ${relationship.key} being ${inspect(resourceIdentifier)}, but ${relationship.key} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(resourceIdentifier));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made this change because relationship.parentType is not a real thing which led to this always reading undefined.


//TODO:Better asserts
return this._internalModelForId(resourceIdentifier.type, resourceIdentifier.id);
Expand All @@ -2634,7 +2669,7 @@ Store = Service.extend({
return;
}

assert(`A ${relationship.parentType} record was pushed into the store with the value of ${relationship.key} being '${inspect(resourceIdentifiers)}', but ${relationship.key} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, Array.isArray(resourceIdentifiers));
assert(`A ${relationship.record.modelName} record was pushed into the store with the value of ${relationship.key} being '${inspect(resourceIdentifiers)}', but ${relationship.key} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, Array.isArray(resourceIdentifiers));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above


let _internalModels = new Array(resourceIdentifiers.length);
for (let i = 0; i < resourceIdentifiers.length; i++) {
Expand Down Expand Up @@ -2674,7 +2709,7 @@ function _commit(adapter, store, operation, snapshot) {
if (adapterPayload) {
payload = normalizeResponseHelper(serializer, store, modelClass, adapterPayload, snapshot.id, operation);
if (payload.included) {
store.push({ data: payload.included });
store._push({ data: null, included: payload.included });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the difference between store.push and store._push here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcardarella the main difference is that it doesn't materialize the records, however it also will likely become the basis for a new public API that's currently going through the RFC process.

}
data = payload.data;
}
Expand All @@ -2695,17 +2730,17 @@ function _commit(adapter, store, operation, snapshot) {
}, label);
}

function setupRelationships(store, record, data) {
function setupRelationships(store, internalModel, data) {
if (!data.relationships) {
return;
}

record.type.eachRelationship((key, descriptor) => {
internalModel.type.eachRelationship((key, descriptor) => {
if (!data.relationships[key]) {
return;
}

let relationship = record._relationships.get(key);
let relationship = internalModel._relationships.get(key);
relationship.push(data.relationships[key]);
});
}
Expand Down
9 changes: 5 additions & 4 deletions addon/-private/system/store/common.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Ember from 'ember';

var get = Ember.get;
const {
get
} = Ember;

const {
__bind,
Expand All @@ -12,9 +14,8 @@ const {
'_objectIsAlive'
);

export function _bind(fn) {
export function _bind(fn, ...args) {
heimdall.increment(__bind);
var args = Array.prototype.slice.call(arguments, 1);

return function() {
return fn.apply(undefined, args);
Expand All @@ -23,7 +24,7 @@ export function _bind(fn) {

export function _guard(promise, test) {
heimdall.increment(__guard);
var guarded = promise['finally'](function() {
let guarded = promise['finally'](function() {
if (!test()) {
guarded._subscribers.length = 0;
}
Expand Down
19 changes: 9 additions & 10 deletions addon/-private/system/store/finders.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ export function _find(adapter, store, typeClass, id, internalModel, options) {
id: 'ds.store.findRecord.id-mismatch'
});

var internalModel = store._push(payload);
return internalModel;
return store._push(payload);
});
}, function(error) {
internalModel.notFound();
Expand Down Expand Up @@ -180,19 +179,19 @@ export function _query(adapter, store, typeClass, query, recordArray) {
}, null, 'DS: Extract payload of query ' + typeClass);
}

export function _queryRecord(adapter, store, typeClass, query) {
var modelName = typeClass.modelName;
var promise = adapter.queryRecord(store, typeClass, query);
var serializer = serializerForAdapter(store, adapter, modelName);
var label = "DS: Handle Adapter#queryRecord of " + typeClass;
export function _queryRecord(adapter, store, modelClass, query) {
let modelName = modelClass.modelName;
let promise = adapter.queryRecord(store, modelClass, query);
let serializer = serializerForAdapter(store, adapter, modelName);
let label = `DS: Handle Adapter#queryRecord of ${modelName}`;

promise = Promise.resolve(promise, label);
promise = _guard(promise, _bind(_objectIsAlive, store));

return promise.then(function(adapterPayload) {
var internalModel;
let internalModel;
store._adapterRun(function() {
var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'queryRecord');
let payload = normalizeResponseHelper(serializer, store, modelClass, adapterPayload, null, 'queryRecord');

assert("Expected the primary data returned by the serializer for a `queryRecord` response to be a single object or null but instead it was an array.", !Array.isArray(payload.data), {
id: 'ds.store.queryRecord-array-response'
Expand All @@ -203,5 +202,5 @@ export function _queryRecord(adapter, store, typeClass, query) {

return internalModel;

}, null, "DS: Extract payload of queryRecord " + typeClass);
}, null, "DS: Extract payload of queryRecord " + modelClass);
}