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

remove needless change events when creating a recordArrays #4947

Merged
merged 10 commits into from
May 16, 2017
26 changes: 17 additions & 9 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,30 @@
language: node_js
sudo: false
node_js:
- "6.9"
- "7"

cache:
yarn: true

before_install:
- "npm config set spin false"
- "npm install -g npm@^2"
- "export PATH=$PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64/bin:$PATH"
- "if [ $(phantomjs --version) != '2.1.1' ]; then rm -rf $PWD/travis_phantomjs; mkdir -p $PWD/travis_phantomjs; fi"
- "if [ $(phantomjs --version) != '2.1.1' ]; then wget https://assets.membergetmember.co/software/phantomjs-2.1.1-linux-x86_64.tar.bz2 -O $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2; fi"
- "if [ $(phantomjs --version) != '2.1.1' ]; then tar -xvf $PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64.tar.bz2 -C $PWD/travis_phantomjs; fi"
- "phantomjs --version"

install:
- npm install --no-optional
- yarn
- ./node_modules/.bin/bower install

script:
- ./bin/lint-features
- npm run-script test
- npm run-script test:optional-features
- npm run-script test:production
- npm run-script node-tests
- yarn run test
- yarn run test:optional-features
- yarn run test:production
- yarn run node-tests
after_success:
- npm run-script production
- yarn run production
- "./bin/publish-builds"
env:
global:
Expand Down
209 changes: 133 additions & 76 deletions addon/-private/system/record-array-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import {
FilteredRecordArray,
AdapterPopulatedRecordArray
} from "./record-arrays";

import cloneNull from "./clone-null";
import { assert } from '@ember/debug';

const {
get,
set,
run: emberRun
} = Ember;

Expand All @@ -25,7 +28,6 @@ const {
createRecordArray,
liveRecordArrayFor,
filteredRecordArraysFor,
populateLiveRecordArray,
recordDidChange,
registerFilteredRecordArray,
unregisterRecordArray,
Expand All @@ -41,7 +43,6 @@ const {
'createRecordArray',
'liveRecordArrayFor',
'filteredRecordArraysFor',
'populateLiveRecordArray',
'recordDidChange',
'registerFilteredRecordArray',
'unregisterRecordArray',
Expand Down Expand Up @@ -126,59 +127,22 @@ export default class RecordArrayManager {
}
}

// TODO: skip if it only changed
// process liveRecordArrays
if (this._liveRecordArrays[modelName]) {
this.updateLiveRecordArray(modelName, internalModels);
let array = this._liveRecordArrays[modelName];
if (array) {
// TODO: skip if it only changed
// process liveRecordArrays
this.updateLiveRecordArray(array, internalModels);
}

// process adapterPopulatedRecordArrays
if (modelsToRemove.length > 0) {
this.removeFromAdapterPopulatedRecordArrays(modelsToRemove);
removeFromAdapterPopulatedRecordArrays(modelsToRemove);
}
}
}

updateLiveRecordArray(modelName, internalModels) {
let array = this.liveRecordArrayFor(modelName);

let modelsToAdd = [];
let modelsToRemove = [];

for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let isDeleted = internalModel.isHiddenFromRecordArrays();
let recordArrays = internalModel._recordArrays;

if (!isDeleted && !internalModel.isEmpty()) {
if (!recordArrays.has(array)) {
modelsToAdd.push(internalModel);
recordArrays.add(array);
}
}

if (isDeleted) {
modelsToRemove.push(internalModel);
recordArrays.delete(array)
}
}

if (modelsToAdd.length > 0) { array._pushInternalModels(modelsToAdd); }
if (modelsToRemove.length > 0) { array._removeInternalModels(modelsToRemove); }
}

removeFromAdapterPopulatedRecordArrays(internalModels) {
for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let list = internalModel._recordArrays.list;

for (let j = 0; j < list.length; j++) {
// TODO: group by arrays, so we can batch remove
list[j]._removeInternalModels([internalModel]);
}

internalModel._recordArrays.clear();
}
updateLiveRecordArray(array, internalModels) {
return updateLiveRecordArray(array, internalModels);
}

/**
Expand Down Expand Up @@ -217,7 +181,7 @@ export default class RecordArrayManager {
}

// TODO: remove, utilize existing flush code but make it flush sync based on 1 modelName
syncLiveRecordArray(array, modelName) {
_syncLiveRecordArray(array, modelName) {
assert(`recordArrayManger.syncLiveRecordArray expects modelName not modelClass as the second param`, typeof modelName === 'string');
let hasNoPotentialDeletions = Object.keys(this._pending).length === 0;
let map = this.store._internalModelsFor(modelName);
Expand All @@ -228,29 +192,19 @@ export default class RecordArrayManager {
liveRecordArrays, and is capable of strategically flushing those changes and applying
small diffs if desired. However, until we've refactored recordArrayManager, this dirty
check prevents us from unnecessarily wiping out live record arrays returned by peekAll.
*/
*/
if (hasNoPotentialDeletions && hasNoInsertionsOrRemovals) {
return;
}

this.populateLiveRecordArray(array, map.models);
}

// TODO: remove, when syncLiveRecordArray is removed
populateLiveRecordArray(array, internalModels) {
heimdall.increment(populateLiveRecordArray);

let internalModels = this._visibleInternalModelsByType(modelName);
let modelsToAdd = [];
for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];

if (!internalModel.isHiddenFromRecordArrays()) {
let recordArrays = internalModel._recordArrays;

if (!recordArrays.has(array)) {
modelsToAdd.push(internalModel);
recordArrays.add(array);
}
let recordArrays = internalModel._recordArrays;
if (recordArrays.has(array) === false) {
recordArrays.add(array);
modelsToAdd.push(internalModel);
}
}

Expand Down Expand Up @@ -278,6 +232,13 @@ export default class RecordArrayManager {
this.updateFilterRecordArray(array, filter, internalModels);
}

_didUpdateAll(modelName) {
let recordArray = this._liveRecordArrays[modelName];
if (recordArray) {
set(recordArray, 'isUpdating', false);
}
}

/**
Get the `DS.RecordArray` for a modelName, which contains all loaded records of
given modelName.
Expand All @@ -291,9 +252,33 @@ export default class RecordArrayManager {

heimdall.increment(liveRecordArrayFor);

return this._liveRecordArrays[modelName] || (this._liveRecordArrays[modelName] = this.createRecordArray(modelName))
let array = this._liveRecordArrays[modelName];

if (array) {
// if the array already exists, synchronize
this._syncLiveRecordArray(array, modelName);
} else {
// if the array is being newly created merely create it with its initial
// content already set. This prevents unneeded change events.
let internalModels = this._visibleInternalModelsByType(modelName);
array = this.createRecordArray(modelName, internalModels);
this._liveRecordArrays[modelName] = array;
}

return array;
}

_visibleInternalModelsByType(modelName) {
let all = this.store._internalModelsFor(modelName)._models;
let visible = [];
for (let i = 0; i < all.length; i++) {
let model = all[i];
if (model.isHiddenFromRecordArrays() === false) {
visible.push(model);
}
}
return visible;
}
/**
Get the `DS.RecordArray` for a modelName, which contains all loaded records of
given modelName.
Expand All @@ -314,18 +299,26 @@ export default class RecordArrayManager {

@method createRecordArray
@param {String} modelName
@param {Array} _content (optional|private)
@return {DS.RecordArray}
*/
createRecordArray(modelName) {
createRecordArray(modelName, content) {
assert(`recordArrayManger.createRecordArray expects modelName not modelClass as the param`, typeof modelName === 'string');
heimdall.increment(createRecordArray);
return RecordArray.create({

let array = RecordArray.create({
modelName,
content: Ember.A(),
content: Ember.A(content || []),
store: this.store,
isLoaded: true,
manager: this
});

if (Array.isArray(content)) {
associateWithRecordArray(content, array);
}

return array;
}

/**
Expand Down Expand Up @@ -363,17 +356,34 @@ export default class RecordArrayManager {
@param {Object} query
@return {DS.AdapterPopulatedRecordArray}
*/
createAdapterPopulatedRecordArray(modelName, query) {
createAdapterPopulatedRecordArray(modelName, query, internalModels, payload) {
heimdall.increment(createAdapterPopulatedRecordArray);
assert(`recordArrayManger.createAdapterPopulatedRecordArray expects modelName not modelClass as the first param, received ${modelName}`, typeof modelName === 'string');

let array = AdapterPopulatedRecordArray.create({
modelName,
query: query,
content: Ember.A(),
store: this.store,
manager: this
});
let array;
if (Array.isArray(internalModels)) {
array = AdapterPopulatedRecordArray.create({
modelName,
query: query,
content: Ember.A(internalModels),
store: this.store,
manager: this,
isLoaded: true,
isUpdating: false,
meta: cloneNull(payload.meta),
links: cloneNull(payload.links)
});

associateWithRecordArray(internalModels, array);
} else {
array = AdapterPopulatedRecordArray.create({
modelName,
query: query,
content: Ember.A(),
store: this.store,
manager: this
});
}

this._adapterPopulatedRecordArrays.push(array);

Expand Down Expand Up @@ -470,3 +480,50 @@ function remove(array, item) {

return false;
}

function updateLiveRecordArray(array, internalModels) {
let modelsToAdd = [];
let modelsToRemove = [];

for (let i = 0; i < internalModels.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does our new diffArray method work here or is the data structure different?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. I didn't think of that, I should try

let internalModel = internalModels[i];
let isDeleted = internalModel.isHiddenFromRecordArrays();
let recordArrays = internalModel._recordArrays;

if (!isDeleted && !internalModel.isEmpty()) {
if (!recordArrays.has(array)) {
modelsToAdd.push(internalModel);
recordArrays.add(array);
}
}

if (isDeleted) {
modelsToRemove.push(internalModel);
recordArrays.delete(array)
}
}

if (modelsToAdd.length > 0) { array._pushInternalModels(modelsToAdd); }
if (modelsToRemove.length > 0) { array._removeInternalModels(modelsToRemove); }
}

function removeFromAdapterPopulatedRecordArrays(internalModels) {
for (let i = 0; i < internalModels.length; i++) {
let internalModel = internalModels[i];
let list = internalModel._recordArrays.list;

for (let j = 0; j < list.length; j++) {
// TODO: group by arrays, so we can batch remove
list[j]._removeInternalModels([internalModel]);
}

internalModel._recordArrays.clear();
}
}

export function associateWithRecordArray(internalModels, array) {
for (let i = 0, l = internalModels.length; i < l; i++) {
let internalModel = internalModels[i];
internalModel._recordArrays.add(array);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Ember from 'ember';
import RecordArray from "./record-array";
import cloneNull from "../clone-null";
import { associateWithRecordArray } from '../record-array-manager';

/**
@module ember-data
Expand Down Expand Up @@ -87,11 +88,7 @@ export default RecordArray.extend({
links: cloneNull(payload.links)
});

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

internalModel._recordArrays.add(this);
}
associateWithRecordArray(internalModels, this);

// TODO: should triggering didLoad event be the last action of the runLoop?
Ember.run.once(this, 'trigger', 'didLoad');
Expand Down
Loading