From 8ea0f0407ab7bf34a1bc2ae9900c244219d98d58 Mon Sep 17 00:00:00 2001 From: Chad Hietala Date: Fri, 11 Dec 2015 14:56:32 -0800 Subject: [PATCH] [PERF] Don't use array methods 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. --- .../serializers/embedded-records-mixin.js | 24 ++++--- .../serializers/json-api-serializer.js | 42 ++++++++++--- addon/-private/serializers/json-serializer.js | 18 ++++-- addon/-private/system/many-array.js | 8 ++- addon/-private/system/model/errors.js | 24 ++++--- addon/-private/system/model/internal-model.js | 10 +-- addon/-private/system/relationships/ext.js | 17 ++--- .../system/relationships/state/has-many.js | 22 ++++--- addon/-private/system/store.js | 62 +++++++++++++------ addon/-private/system/store/finders.js | 20 +++--- 10 files changed, 173 insertions(+), 74 deletions(-) diff --git a/addon/-private/serializers/embedded-records-mixin.js b/addon/-private/serializers/embedded-records-mixin.js index e7968bd13f6..579ba8331e4 100644 --- a/addon/-private/serializers/embedded-records-mixin.js +++ b/addon/-private/serializers/embedded-records-mixin.js @@ -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, l = manyArray.length; i < l; 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; }, /** @@ -450,11 +456,15 @@ export default Ember.Mixin.create({ */ _extractEmbeddedHasMany(store, key, hash, relationshipMeta) { let relationshipHash = get(hash, `data.relationships.${key}.data`); + if (!relationshipHash) { return; } - let hasMany = relationshipHash.map(item => { + let hasMany = new Array(relationshipHash.length); + + 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); @@ -462,8 +472,8 @@ export default Ember.Mixin.create({ hash.included.push(...included); } - return { id: data.id, type: data.type }; - }); + hasMany[i] = { id: data.id, type: data.type }; + } let relationship = { data: hasMany }; set(hash, `data.relationships.${key}`, relationship); diff --git a/addon/-private/serializers/json-api-serializer.js b/addon/-private/serializers/json-api-serializer.js index 1bcb674753f..5f56a1c65bc 100644 --- a/addon/-private/serializers/json-api-serializer.js +++ b/addon/-private/serializers/json-api-serializer.js @@ -108,12 +108,26 @@ 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); + } else if (Array.isArray(documentHash.data)) { + 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); + if (Array.isArray(documentHash.included)) { + 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; @@ -215,8 +229,15 @@ const JSONAPISerializer = JSONSerializer.extend({ relationshipHash.data = this._normalizeRelationshipDataHelper(relationshipHash.data); } - if (Ember.typeOf(relationshipHash.data) === 'array') { - relationshipHash.data = relationshipHash.data.map(this._normalizeRelationshipDataHelper, this); + if (Array.isArray(relationshipHash.data)) { + 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; @@ -456,12 +477,15 @@ const JSONAPISerializer = JSONSerializer.extend({ payloadKey = this.keyForRelationship(key, 'hasMany', 'serialize'); } - let data = hasMany.map((item) => { - return { + let data = new Array(hasMany.length); + + for (let i = 0, l = hasMany.length; i < l; i++) { + let item = hasMany[i]; + data[i] = { type: this.payloadKeyFromModelName(item.modelName), id: item.id }; - }); + } json.relationships[payloadKey] = { data }; } diff --git a/addon/-private/serializers/json-serializer.js b/addon/-private/serializers/json-serializer.js index af32f6a727f..b43aafb0dc9 100644 --- a/addon/-private/serializers/json-serializer.js +++ b/addon/-private/serializers/json-serializer.js @@ -453,13 +453,17 @@ 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; @@ -644,7 +648,13 @@ 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)); + if (!Ember.isNone(relationshipHash)) { + data = new Array(relationshipHash.length); + for (let i = 0, l = relationshipHash.length; i < l; i++) { + let item = relationshipHash[i]; + data[i] = this.extractRelationship(relationshipMeta.type, item); + } + } } relationship = { data }; } diff --git a/addon/-private/system/many-array.js b/addon/-private/system/many-array.js index bf09a7505dc..c75c4790a91 100644 --- a/addon/-private/system/many-array.js +++ b/addon/-private/system/many-array.js @@ -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); } }, /** diff --git a/addon/-private/system/model/errors.js b/addon/-private/system/model/errors.js index 09c87ed628e..14ce947449b 100644 --- a/addon/-private/system/model/errors.js +++ b/addon/-private/system/model/errors.js @@ -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; }, /** diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index b28b2937a80..23f1f5f5ce3 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -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 = new Array(preloadValue.length); + + for (let i = 0, l = preloadValue.length; i < l; i++) { + let recordToPush = preloadValue[i]; + recordsToSet[i] = 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); diff --git a/addon/-private/system/relationships/ext.js b/addon/-private/system/relationships/ext.js index 87277e5aaeb..6ae9358e806 100644 --- a/addon/-private/system/relationships/ext.js +++ b/addon/-private/system/relationships/ext.js @@ -554,7 +554,7 @@ Model.reopenClass({ eachRelationship(callback, binding) { get(this, 'relationshipsByName').forEach((relationship, name) => { callback.call(binding, name, relationship); - }); + }) }, /** @@ -569,16 +569,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'; diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index b9f8296e107..ca8bde09911 100644 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -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); diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index e09f41c8734..ae49b77e444 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -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")); }, /** @@ -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 Ember.RSVP.Promise.all(fetches); }, scheduleFetch(internalModel, options) { @@ -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._findByInternalModel(internalModels[i])); + } + + return Promise.all(finds); }, @@ -1098,19 +1115,22 @@ 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 @@ -1118,10 +1138,6 @@ Store = Service.extend({ typeMap.metadata = new EmptyObject(); } - - function byType(entry) { - return typeMaps[entry]['type'].modelName; - } }, /** @@ -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 diff --git a/addon/-private/system/store/finders.js b/addon/-private/system/store/finders.js index e032ad83b83..6962cb1f63f 100644 --- a/addon/-private/system/store/finders.js +++ b/addon/-private/system/store/finders.js @@ -54,10 +54,10 @@ export function _find(adapter, store, typeClass, id, internalModel, options) { export function _findMany(adapter, store, typeClass, ids, internalModels) { - var snapshots = Ember.A(internalModels).invoke('createSnapshot'); - var promise = adapter.findMany(store, typeClass, ids, snapshots); - var serializer = serializerForAdapter(store, adapter, typeClass.modelName); - var label = "DS: Handle Adapter#findMany of " + typeClass; + let snapshots = Ember.A(internalModels).invoke('createSnapshot'); + let promise = adapter.findMany(store, typeClass, ids, snapshots); + let serializer = serializerForAdapter(store, adapter, typeClass.modelName); + let label = "DS: Handle Adapter#findMany of " + typeClass; if (promise === undefined) { throw new Error('adapter.findMany returned undefined, this was very likely a mistake'); @@ -69,10 +69,16 @@ export function _findMany(adapter, store, typeClass, ids, internalModels) { return promise.then(function(adapterPayload) { assert("You made a `findMany` request for a " + typeClass.typeClassKey + " with ids " + ids + ", but the adapter's response did not have any data", payloadIsNotBlank(adapterPayload)); return store._adapterRun(function() { - var payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'findMany'); + let payload = normalizeResponseHelper(serializer, store, typeClass, adapterPayload, null, 'findMany'); //TODO Optimize, no need to materialize here - var records = store.push(payload); - return records.map((record) => record._internalModel); + let records = store.push(payload); + let internalModels = new Array(records.length); + + for (let i = 0, l = records.length; i < l; i++) { + internalModels[i] = records[i]._internalModel; + } + + return internalModels; }); }, null, "DS: Extract payload of " + typeClass); }