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] minor adjustments to store#push flow #3889

Merged
merged 2 commits into from
Oct 26, 2015

Conversation

trentmwillis
Copy link
Member

Some simple suggestions of refactoring to squeeze out a little extra performance when calling store#push. In our application this shows ~50ms improvement for slower devices on medium-sized payloads. Changes are as follows:

  • Don't call coerceId in _load since it gets called in _internalModelForId
  • Use Ember.isArray instead of Ember.typeOf
  • Use for-loops instead of Array.prototype methods

@stefanpenner
Copy link
Member

We should prefer Array.isArray over Ember.isArray as the later is merely an alias for the former and exists for compar

var internalModels = data.data.map((recordData) => this._pushInternalModel(recordData));
return internalModels.map((internalModel) => internalModel.getRecord());
if (isArray(data.data)) {
var internalModels = [];
Copy link
Member

Choose a reason for hiding this comment

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

new Array(length)

@stefanpenner
Copy link
Member

Two comments otherwise this Lgtm

@trentmwillis
Copy link
Member Author

@stefanpenner updated, looks like AppVeyor had issues with installing node modules though

@stefanpenner
Copy link
Member

@trentmwillis silly appveyor...

anyways thanks this looks good to me. Its really sad that forEach & map have such a silly high cost..

stefanpenner added a commit that referenced this pull request Oct 26, 2015
 [perf] minor adjustments to `store#push` flow
@stefanpenner stefanpenner merged commit 023d4c7 into emberjs:master Oct 26, 2015
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.

2 participants