Skip to content

Commit

Permalink
[PERF] Don't use array methods
Browse files Browse the repository at this point in the history
We shouldn't use array methods as they allocate and close over items that need to be GC'd. Memory preasure can be an issue and we should do everything to reduce that preasure.
  • Loading branch information
chadhietala committed Dec 11, 2015
1 parent 209326f commit 465ff85
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 74 deletions.
22 changes: 15 additions & 7 deletions addon/-private/serializers/embedded-records-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,17 @@ export default Ember.Mixin.create({
*/
_generateSerializedHasMany(snapshot, relationship) {
let hasMany = snapshot.hasMany(relationship.key);
return Ember.A(hasMany).map((embeddedSnapshot) => {
var embeddedJson = embeddedSnapshot.record.serialize({ includeId: true });
let manyArray = Ember.A(hasMany);
let ret = new Array(manyArray.length);

for (let i = 0; i < manyArray.length; i++) {
let embeddedSnapshot = manyArray[i];
let embeddedJson = embeddedSnapshot.record.serialize({ includeId: true });
this.removeEmbeddedForeignKey(snapshot, embeddedSnapshot, relationship, embeddedJson);
return embeddedJson;
});
ret[i] = embeddedJson;
}

return ret;
},

/**
Expand Down Expand Up @@ -450,20 +456,22 @@ export default Ember.Mixin.create({
*/
_extractEmbeddedHasMany(store, key, hash, relationshipMeta) {
let relationshipHash = get(hash, `data.relationships.${key}.data`);
let hasMany = [];
if (!relationshipHash) {
return;
}

let hasMany = relationshipHash.map(item => {
for (let i = 0, l = relationshipHash.length; i < l; i++) {
let item = relationshipHash[i];
let { data, included } = this._normalizeEmbeddedRelationship(store, relationshipMeta, item);
hash.included = hash.included || [];
hash.included.push(data);
if (included) {
hash.included.push(...included);
}

return { id: data.id, type: data.type };
});
hasMany.push({ id: data.id, type: data.type });
}

let relationship = { data: hasMany };
set(hash, `data.relationships.${key}`, relationship);
Expand Down
38 changes: 31 additions & 7 deletions addon/-private/serializers/json-api-serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,25 @@ const JSONAPISerializer = JSONSerializer.extend({
if (Ember.typeOf(documentHash.data) === 'object') {
documentHash.data = this._normalizeResourceHelper(documentHash.data);
} else if (Ember.typeOf(documentHash.data) === 'array') {
documentHash.data = documentHash.data.map(this._normalizeResourceHelper, this);
let ret = new Array(documentHash.data.length);

for (let i = 0, l = documentHash.data.length; i < l; i++) {
let data = documentHash.data[i];
ret[i] = this._normalizeResourceHelper(data);
}

documentHash.data = ret;
}

if (Ember.typeOf(documentHash.included) === 'array') {
documentHash.included = documentHash.included.map(this._normalizeResourceHelper, this);
let ret = new Array(documentHash.included.length);

for (let i = 0, l = documentHash.included.length; i < l; i++) {
let included = documentHash.included[i];
ret[i] = this._normalizeResourceHelper(included);
}

documentHash.included = ret;
}

return documentHash;
Expand Down Expand Up @@ -216,7 +230,14 @@ const JSONAPISerializer = JSONSerializer.extend({
}

if (Ember.typeOf(relationshipHash.data) === 'array') {
relationshipHash.data = relationshipHash.data.map(this._normalizeRelationshipDataHelper, this);
let ret = new Array(relationshipHash.data.length);

for (let i = 0, l = relationshipHash.data.length; i < l; i++) {
let data = relationshipHash.data[i];
ret[i] = this._normalizeRelationshipDataHelper(data);
}

relationshipHash.data = ret;
}

return relationshipHash;
Expand Down Expand Up @@ -456,12 +477,15 @@ const JSONAPISerializer = JSONSerializer.extend({
payloadKey = this.keyForRelationship(key, 'hasMany', 'serialize');
}

let data = hasMany.map((item) => {
return {
let data = [];

for (let i = 0, l = hasMany.length; i < l; i++) {
let item = hasMany[i];
data.push({
type: this.payloadKeyFromModelName(item.modelName),
id: item.id
};
});
});
}

json.relationships[payloadKey] = { data };
}
Expand Down
21 changes: 17 additions & 4 deletions addon/-private/serializers/json-serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -453,13 +453,18 @@ export default Serializer.extend({
documentHash.included = included;
}
} else {
documentHash.data = payload.map((item) => {
let ret = new Array(payload.length);

for (let i = 0, l = payload.length; i < l; i++) {
let item = payload[i];
let { data, included } = this.normalize(primaryModelClass, item);
if (included) {
documentHash.included.push(...included);
}
return data;
});
ret[i] = data;
}

documentHash.data = ret;
}

return documentHash;
Expand Down Expand Up @@ -644,7 +649,15 @@ export default Serializer.extend({
data = this.extractRelationship(relationshipMeta.type, relationshipHash);
}
} else if (relationshipMeta.kind === 'hasMany') {
data = Ember.isNone(relationshipHash) ? null : relationshipHash.map((item) => this.extractRelationship(relationshipMeta.type, item));
let data = null;

if (!Ember.isNone(relationshipHash)) {
data = [];
for (let i = 0, l = relationshipHash.length; i < l; i++) {
let item = relationshipHash[i];
data.push(this.extractRelationship(relationshipMeta.type, item));
}
}
}
relationship = { data };
}
Expand Down
8 changes: 7 additions & 1 deletion addon/-private/system/many-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,13 @@ export default Ember.Object.extend(Ember.MutableArray, Ember.Evented, {
this.get('relationship').removeRecords(records);
}
if (objects) {
this.get('relationship').addRecords(objects.map((obj) => obj._internalModel), idx);
let internalModels = new Array(objects.length);

for (let i = 0, l = objects.length; i < l; i++) {
internalModels[i] = objects[i]._internalModel;
}

this.get('relationship').addRecords(internalModels, idx);
}
},
/**
Expand Down
24 changes: 17 additions & 7 deletions addon/-private/system/model/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,24 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
@private
*/
_findOrCreateMessages(attribute, messages) {
var errors = this.errorsFor(attribute);
let errors = this.errorsFor(attribute);
let _messages = [];
let messagesArray = makeArray(messages);

for (let i = 0, l = messagesArray.length; i < l; i++) {
let message = messagesArray[i];
let err = errors.findBy('message', message);
if (err) {
_messages.push(err);
} else {
_messages.push({
attribute: attribute,
message: message
});
}
}

return makeArray(messages).map((message) => {
return errors.findBy('message', message) || {
attribute: attribute,
message: message
};
});
return _messages;
},

/**
Expand Down
10 changes: 6 additions & 4 deletions addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,11 +566,13 @@ InternalModel.prototype = {

_preloadHasMany(key, preloadValue, type) {
assert("You need to pass in an array to set a hasMany property on a record", Ember.isArray(preloadValue));
var internalModel = this;
let recordsToSet = [];

for (let i = 0, l = preloadValue.length; i < l; i++) {
let recordToPush = preloadValue[i];
recordsToSet.push(this._convertStringOrNumberIntoInternalModel(recordToPush, type));
}

var recordsToSet = preloadValue.map((recordToPush) => {
return internalModel._convertStringOrNumberIntoInternalModel(recordToPush, type);
});
//We use the pathway of setting the hasMany as if it came from the adapter
//because the user told us that they know this relationships exists already
this._relationships.get(key).updateRecordsFromAdapter(recordsToSet);
Expand Down
24 changes: 15 additions & 9 deletions addon/-private/system/relationships/ext.js
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,12 @@ Model.reopenClass({
@param {any} binding the value to which the callback's `this` should be bound
*/
eachRelationship(callback, binding) {
get(this, 'relationshipsByName').forEach((relationship, name) => {
callback.call(binding, name, relationship);
});
let relationships = get(this, 'relationshipsByName');

for (let i = 0, l = relationships.length; i < l; i++) {
let rel = relationships[i];
callback.call(binding, i, rel);
}
},

/**
Expand All @@ -569,16 +572,19 @@ Model.reopenClass({
@param {any} binding the value to which the callback's `this` should be bound
*/
eachRelatedType(callback, binding) {
get(this, 'relatedTypes').forEach((type) => {
let relationshipTypes = get(this, 'relatedTypes');

for (let i = 0; i < relationshipTypes.length; i++) {
let type = relationshipTypes[i];
callback.call(binding, type);
});
}
},

determineRelationshipType(knownSide, store) {
var knownKey = knownSide.key;
var knownKind = knownSide.kind;
var inverse = this.inverseFor(knownKey, store);
var key, otherKind;
let knownKey = knownSide.key;
let knownKind = knownSide.kind;
let inverse = this.inverseFor(knownKey, store);
let key, otherKind;

if (!inverse) {
return knownKind === 'belongsTo' ? 'oneToNone' : 'manyToNone';
Expand Down
22 changes: 14 additions & 8 deletions addon/-private/system/relationships/state/has-many.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,21 @@ ManyRelationship.prototype.fetchLink = function() {
};

ManyRelationship.prototype.findRecords = function() {
let manyArray = this.manyArray.toArray();
let internalModels = new Array(manyArray.length);

for (let i = 0, l = manyArray.length; i < l; i++) {
internalModels[i] = manyArray[i].internalModel;
}

//TODO CLEANUP
return this.store.findMany(this.manyArray.toArray().map((rec) => rec._internalModel)).
then(() => {
if (!this.manyArray.get('isDestroyed')) {
//Goes away after the manyArray refactor
this.manyArray.set('isLoaded', true);
}
return this.manyArray;
});
return this.store.findMany(internalModels).then(() => {
if (!this.manyArray.get('isDestroyed')) {
//Goes away after the manyArray refactor
this.manyArray.set('isLoaded', true);
}
return this.manyArray;
});
};
ManyRelationship.prototype.notifyHasManyChanged = function() {
this.record.notifyHasManyAdded(this.key);
Expand Down
62 changes: 42 additions & 20 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -578,11 +578,13 @@ Store = Service.extend({
*/
findByIds(modelName, ids) {
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), typeof modelName === 'string');
var store = this;
let promises = [];

for (let i = 0, l = ids.length; i < l; i++) {
promises.push(this.findRecord(modelName, ids[i]));
}

return promiseArray(Ember.RSVP.all(ids.map((id) => {
return store.findRecord(modelName, id);
})).then(Ember.A, null, "DS: Store#findByIds of " + modelName + " complete"));
return promiseArray(Ember.RSVP.all(promises).then(Ember.A, null, "DS: Store#findByIds of " + modelName + " complete"));
},

/**
Expand All @@ -609,8 +611,17 @@ Store = Service.extend({
},

scheduleFetchMany(records) {
var internalModels = records.map((record) => record._internalModel);
return Promise.all(internalModels.map(this.scheduleFetch, this));
let internalModels = new Array(records.length);
let fetches = new Array(records.length);
for (let i = 0, l = records.length; i < l; i++) {
internalModels[i] = records[i]._internalModel;
}

for (let i = 0, l = internalModels.length; i < l; i++) {
fetches[i] = this.scheduleFetch(internalModels[i]);
}

return Promise.all(fetches);
},

scheduleFetch(internalModel, options) {
Expand Down Expand Up @@ -841,7 +852,13 @@ Store = Service.extend({
@return {Promise} promise
*/
findMany(internalModels) {
return Promise.all(internalModels.map((internalModel) => this._findByInternalModel(internalModel)));
let finds = [];

for (let i = 0, l = internalModels.length; i < l; i++) {
finds.push(this._findInternalModel(internalModels[i]));
}

return Promise.all(finds);
},


Expand Down Expand Up @@ -1098,30 +1115,29 @@ Store = Service.extend({
unloadAll(modelName) {
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ Ember.inspect(modelName), !modelName || typeof modelName === 'string');
if (arguments.length === 0) {
var typeMaps = this.typeMaps;
var keys = Object.keys(typeMaps);
let typeMaps = this.typeMaps;
let keys = Object.keys(typeMaps);
let types = new Array(keys.length);

var types = keys.map(byType);
for (let i = 0, l = keys.length; i < l; i++) {
types[i] = typeMaps[keys[i]]['type'].modelName;
}

types.forEach(this.unloadAll, this);
} else {
var typeClass = this.modelFor(modelName);
var typeMap = this.typeMapFor(typeClass);
var records = typeMap.records.slice();
var record;
let typeClass = this.modelFor(modelName);
let typeMap = this.typeMapFor(typeClass);
let records = typeMap.records.slice();
let record;

for (var i = 0; i < records.length; i++) {
for (let i = 0; i < records.length; i++) {
record = records[i];
record.unloadRecord();
record.destroy(); // maybe within unloadRecord
}

typeMap.metadata = new EmptyObject();
}

function byType(entry) {
return typeMaps[entry]['type'].modelName;
}
},

/**
Expand Down Expand Up @@ -2054,8 +2070,14 @@ function deserializeRecordIds(store, key, relationship, ids) {
return;
}

let _ids = [];

for (let i = 0, l = ids.length; i < l; i++) {
_ids.push(deserializeRecordId(store, key, relationship, ids[i]));
}

assert(`A ${relationship.parentType} record was pushed into the store with the value of ${key} being '${Ember.inspect(ids)}', but ${key} is a hasMany relationship so the value must be an array. You should probably check your data payload or serializer.`, isArray(ids));
return ids.map((id) => deserializeRecordId(store, key, relationship, id));
return _ids;
}

// Delegation to the adapter and promise management
Expand Down
Loading

0 comments on commit 465ff85

Please sign in to comment.