Skip to content

Commit

Permalink
perf: optimize unload and relationships (#8149)
Browse files Browse the repository at this point in the history
* improve unload perf

* improve has-many population perf

* fix tests

* remove useless test

* dont double populate

* 2ms is 2ms

* final tweaks and test fixes

* fix stubborn test
  • Loading branch information
runspired authored Aug 25, 2022
1 parent 07e4a71 commit a1fe9d1
Show file tree
Hide file tree
Showing 17 changed files with 238 additions and 252 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,7 @@ module('autotracking has-many', function (hooks) {

names = findAll('li').map((e) => e.textContent);
assert.deepEqual(names, ['RGB', 'RGB'], 'rendered 2 children');
assert.expectDeprecation({ id: 'ember-data:no-a-with-array-like', count: 6 });
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.expect(6);

let { owner } = this;
let belongsToReturnValue = { data: { id: '1', type: 'person' } };
let belongsToReturnValue;

class RelationshipRecordData extends TestRecordData {
getBelongsTo(key: string) {
Expand Down Expand Up @@ -570,6 +570,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
belongsToReturnValue = { data: store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }) };

store.push({
data: [davidHash, runspiredHash],
Expand All @@ -591,7 +592,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.expect(4);
let { owner } = this;

let belongsToReturnValue = { data: { id: '1', type: 'person' } };
let belongsToReturnValue;

let RelationshipRecordData;
if (V2CACHE_SINGLETON_MANAGER) {
Expand All @@ -607,7 +608,9 @@ module('integration/record-data - Custom RecordData Implementations', function (
key: string,
value: StableRecordIdentifier | null
) {
belongsToReturnValue = { data: { id: '3', type: 'person' } };
belongsToReturnValue = {
data: store.identifierCache.getOrCreateRecordIdentifier({ id: '3', type: 'person' }),
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'landlord');
}
};
Expand All @@ -619,7 +622,9 @@ module('integration/record-data - Custom RecordData Implementations', function (
}

setDirtyBelongsTo(this: V1TestRecordData, key: string, recordData: this | null) {
belongsToReturnValue = { data: { id: '3', type: 'person' } };
belongsToReturnValue = {
data: store.identifierCache.getOrCreateRecordIdentifier({ id: '3', type: 'person' }),
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'landlord');
}
};
Expand All @@ -638,6 +643,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
belongsToReturnValue = { data: store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }) };

store.push({
data: [davidHash, runspiredHash, igorHash],
Expand All @@ -662,8 +668,7 @@ module('integration/record-data - Custom RecordData Implementations', function (

let { owner } = this;

let hasManyReturnValue = { data: [{ id: '1', type: 'person' }] };

let hasManyReturnValue;
class RelationshipRecordData extends TestRecordData {
getHasMany(key: string) {
return hasManyReturnValue;
Expand Down Expand Up @@ -716,6 +721,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
hasManyReturnValue = { data: [store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' })] };

store.push({
data: [davidHash, runspiredHash, igorHash],
Expand Down Expand Up @@ -747,8 +753,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.expect(10);
let { owner } = this;

let hasManyReturnValue = { data: [{ id: '1', type: 'person' }] };

let hasManyReturnValue;
class RelationshipRecordData extends TestRecordData {
getHasMany(key: string) {
return hasManyReturnValue;
Expand All @@ -770,8 +775,8 @@ module('integration/record-data - Custom RecordData Implementations', function (

hasManyReturnValue = {
data: [
{ id: '3', type: 'person' },
{ id: '2', type: 'person' },
store.identifierCache.getOrCreateRecordIdentifier({ id: '3', type: 'person' }),
store.identifierCache.getOrCreateRecordIdentifier({ id: '2', type: 'person' }),
],
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
Expand All @@ -787,7 +792,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.strictEqual(key, 'tenants', 'Passed correct key to removeFromHasMany');
assert.strictEqual(recordDatas[0].getResourceIdentifier().id, '2', 'Passed correct RD to removeFromHasMany');
}
hasManyReturnValue = { data: [{ id: '1', type: 'person' }] };
hasManyReturnValue = { data: [store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' })] };
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
}

Expand All @@ -796,8 +801,8 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.strictEqual(recordDatas[0].getResourceIdentifier().id, '3', 'Passed correct RD to addToHasMany');
hasManyReturnValue = {
data: [
{ id: '1', type: 'person' },
{ id: '2', type: 'person' },
store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }),
store.identifierCache.getOrCreateRecordIdentifier({ id: '2', type: 'person' }),
],
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
Expand All @@ -813,8 +818,8 @@ module('integration/record-data - Custom RecordData Implementations', function (
assert.strictEqual(values[0].id, '3', 'Passed correct RD to addToHasMany');
hasManyReturnValue = {
data: [
{ id: '1', type: 'person' },
{ id: '2', type: 'person' },
store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' }),
store.identifierCache.getOrCreateRecordIdentifier({ id: '2', type: 'person' }),
],
};
this._storeWrapper.notifyChange(this._identifier, 'relationships', 'tenants');
Expand All @@ -834,6 +839,7 @@ module('integration/record-data - Custom RecordData Implementations', function (
owner.register('service:store', TestStore);

store = owner.lookup('service:store');
hasManyReturnValue = { data: [store.identifierCache.getOrCreateRecordIdentifier({ id: '1', type: 'person' })] };

store.push({
data: [davidHash, runspiredHash, igorHash],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { settled } from '@ember/test-helpers';

import { module, test } from 'qunit';
import { resolve } from 'rsvp';

Expand Down Expand Up @@ -85,6 +87,7 @@ module('Store.createRecord() coverage', function (hooks) {
assert.deepEqual(pets, ['Shen'], 'Precondition: Chris has Shen as a pet');

pet.unloadRecord();
await settled();
assert.strictEqual(pet.owner, null, 'Shen no longer has an owner');
// check that the relationship has been dissolved
pets = chris.pets.toArray().map((pet) => pet.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1614,7 +1614,7 @@ module('integration/relationships/one_to_many_test - OneToMany relationships', f

deprecatedTest(
'createRecord updates inverse record array which has observers',
{ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 5 },
{ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 4 },
async function (assert) {
let store = this.owner.lookup('service:store');
let adapter = store.adapterFor('application');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import EmberObject, { computed } from '@ember/object';
import { filterBy } from '@ember/object/computed';
import { settled } from '@ember/test-helpers';

import { module, test } from 'qunit';
import { module } from 'qunit';

import { setupRenderingTest } from 'ember-qunit';

Expand All @@ -14,39 +14,43 @@ import { deprecatedTest } from '@ember-data/unpublished-test-infra/test-support/
module('PromiseManyArray', (hooks) => {
setupRenderingTest(hooks);

test('PromiseManyArray is not side-affected by EmberArray', async function (assert) {
const { owner } = this;
class Person extends Model {
@attr('string') name;
}
class Group extends Model {
@hasMany('person', { async: true, inverse: null }) members;
}
owner.register('model:person', Person);
owner.register('model:group', Group);
const store = owner.lookup('service:store');
const members = ['Bob', 'John', 'Michael', 'Larry', 'Lucy'].map((name) => store.createRecord('person', { name }));
const group = store.createRecord('group', { members });

const forEachFn = group.members.forEach;
assert.strictEqual(group.members.length, 5, 'initial length is correct');

if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 4, 'updated length is correct');
}
deprecatedTest(
'PromiseManyArray is not side-affected by EmberArray',
{ id: 'ember-data:no-a-with-array-like', until: '5.0', count: 1 },
async function (assert) {
const { owner } = this;
class Person extends Model {
@attr('string') name;
}
class Group extends Model {
@hasMany('person', { async: true, inverse: null }) members;
}
owner.register('model:person', Person);
owner.register('model:group', Group);
const store = owner.lookup('service:store');
const members = ['Bob', 'John', 'Michael', 'Larry', 'Lucy'].map((name) => store.createRecord('person', { name }));
const group = store.createRecord('group', { members });

A(group.members);
const forEachFn = group.members.forEach;
assert.strictEqual(group.members.length, 5, 'initial length is correct');

assert.strictEqual(forEachFn, group.members.forEach, 'we have the same function for forEach');
if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 4, 'updated length is correct');
}

A(group.members);

assert.strictEqual(forEachFn, group.members.forEach, 'we have the same function for forEach');

if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 3, 'updated length is correct');
// we'll want to use a different test for this but will want to still ensure we are not side-affected
assert.expectDeprecation({ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 2 });
if (DEPRECATE_PROMISE_MANY_ARRAY_BEHAVIORS) {
group.members.replace(0, 1);
assert.strictEqual(group.members.length, 3, 'updated length is correct');
// we'll want to use a different test for this but will want to still ensure we are not side-affected
assert.expectDeprecation({ id: 'ember-data:deprecate-promise-many-array-behaviors', until: '5.0', count: 2 });
}
}
});
);

deprecatedTest(
'PromiseManyArray can be subscribed to by computed chains',
Expand Down Expand Up @@ -130,6 +134,7 @@ module('PromiseManyArray', (hooks) => {
assert.strictEqual(memberIds.length, 6, 'memberIds length is correct');
assert.strictEqual(johnRecords.length, 2, 'johnRecords length is correct');
assert.strictEqual(group.members.length, 6, 'members length is correct');
assert.expectDeprecation({ id: 'ember-data:no-a-with-array-like', count: 2 });
}
);
});
116 changes: 0 additions & 116 deletions packages/-ember-data/tests/unit/many-array-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ import { run } from '@ember/runloop';
import { module, test } from 'qunit';
import { resolve } from 'rsvp';

import { gte } from 'ember-compatibility-helpers';
import { setupTest } from 'ember-qunit';

import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
import { ManyArray } from '@ember-data/model/-private';

module('unit/many_array - ManyArray', function (hooks) {
setupTest(hooks);
Expand Down Expand Up @@ -76,118 +74,4 @@ module('unit/many_array - ManyArray', function (hooks) {
});
});
});

if (!gte('4.0.0')) {
test('manyArray trigger arrayContentChange functions with the correct values', function (assert) {
assert.expect(6);
class Post extends Model {
@attr('string') title;
@hasMany('tag', { async: false, inverse: 'post' }) tags;
}

class Tag extends Model {
@attr('string') name;
@belongsTo('post', { async: false, inverse: 'tags' }) post;
}

this.owner.register('model:post', Post);
this.owner.register('model:tag', Tag);
const store = this.owner.lookup('service:store');

const TestManyArray = ManyArray.proto();

let willChangeStartIdx;
let willChangeRemoveAmt;
let willChangeAddAmt;

let originalArrayContentWillChange = TestManyArray.arrayContentWillChange;
let originalArrayContentDidChange = TestManyArray.arrayContentDidChange;
let originalInit = TestManyArray.init;

// override ManyArray temp (cleanup occures in afterTest);

TestManyArray.init = function (...args) {
// We aren't actually adding any observers in this test
// just testing the observer codepaths, so we use this to
// force the ManyArray instance to take the observer paths.
this.__hasArrayObservers = true;
originalInit.call(this, ...args);
};

TestManyArray.arrayContentWillChange = function (startIdx, removeAmt, addAmt) {
willChangeStartIdx = startIdx;
willChangeRemoveAmt = removeAmt;
willChangeAddAmt = addAmt;

return originalArrayContentWillChange.apply(this, arguments);
};

TestManyArray.arrayContentDidChange = function (startIdx, removeAmt, addAmt) {
assert.strictEqual(startIdx, willChangeStartIdx, 'WillChange and DidChange startIdx should match');
assert.strictEqual(removeAmt, willChangeRemoveAmt, 'WillChange and DidChange removeAmt should match');
assert.strictEqual(addAmt, willChangeAddAmt, 'WillChange and DidChange addAmt should match');

return originalArrayContentDidChange.apply(this, arguments);
};

try {
run(() => {
store.push({
data: [
{
type: 'tag',
id: '1',
attributes: {
name: 'Ember.js',
},
},
{
type: 'tag',
id: '2',
attributes: {
name: 'Tomster',
},
},
{
type: 'post',
id: '3',
attributes: {
title: 'A framework for creating ambitious web applications',
},
relationships: {
tags: {
data: [{ type: 'tag', id: '1' }],
},
},
},
],
});

store.peekRecord('post', 3).tags;

store.push({
data: {
type: 'post',
id: '3',
attributes: {
title: 'A framework for creating ambitious web applications',
},
relationships: {
tags: {
data: [
{ type: 'tag', id: '1' },
{ type: 'tag', id: '2' },
],
},
},
},
});
});
} finally {
TestManyArray.arrayContentWillChange = originalArrayContentWillChange;
TestManyArray.arrayContentDidChange = originalArrayContentDidChange;
TestManyArray.init = originalInit;
}
});
}
});
Loading

0 comments on commit a1fe9d1

Please sign in to comment.