Skip to content

Commit

Permalink
Free up internalModels
Browse files Browse the repository at this point in the history
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
  • Loading branch information
hjdivad committed Oct 22, 2016
1 parent 7c1d52f commit 484909c
Show file tree
Hide file tree
Showing 10 changed files with 236 additions and 104 deletions.
137 changes: 124 additions & 13 deletions addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ function extractPivotName(name) {
);
}

function areAllModelsUnloaded(models) {
for (let i=0; i<models.length; ++i) {
let record = models[i].record;
if (record && !(record.get('isDestroyed') || record.get('isDestroying'))) {
return false;
}
}
return true;
}

// this (and all heimdall instrumentation) will be stripped by a babel transform
// https://github.com/heimdalljs/babel5-plugin-strip-heimdall
const {
Expand Down Expand Up @@ -87,6 +97,8 @@ const {
'updateChangedAttributes'
);

let nextBfsId = 1;

/*
`InternalModel` is the Model class that we use internally inside Ember Data to represent models.
Internal ED methods should only deal with `InternalModel` objects. It is a fast, plain Javascript class.
Expand All @@ -111,28 +123,28 @@ export default class InternalModel {
this.store = store;
this._data = data || new EmptyObject();
this.modelName = type.modelName;
this.dataHasInitialized = false;
this._loadingPromise = null;
this._recordArrays = undefined;
this.currentState = RootState.empty;
this.isReloading = false;
this._isDestroying = false;
this._isDestroyed = false;
this.isError = false;
this.error = null;

this.resetRecord();

// caches for lazy getters
this.__deferredTriggers = null;
this._references = null;
this._recordReference = null;
this.__inFlightAttributes = null;
this.__relationships = null;
this.__attributes = null;
this.__implicitRelationships = null;

// Used for coloring during BFS
this._bfsId = 0;
}

get recordReference() {
if (this._recordReference === null) {
this._recordReference = new RecordReference(this.store, this)
this._recordReference = new RecordReference(this.store, this);
}
return this._recordReference;
}
Expand Down Expand Up @@ -278,8 +290,22 @@ export default class InternalModel {
this._triggerDeferredTriggers();
}

recordObjectWillDestroy() {
resetRecord() {
this.record = null;
this.dataHasInitialized = false;
this.isReloading = false;
this.error = null;
this.currentState = RootState.empty;
this.__attributes = null;
this.__inFlightAttributes = null;
}

dematerializeRecord() {
if (this.record) {
this.record.destroy();
this.clearRelationships();
this.updateRecordArrays();
}
}

deleteRecord() {
Expand Down Expand Up @@ -333,14 +359,102 @@ export default class InternalModel {
return this.record;
}


/**
Computes the set of internal models reachable from `this` acrosxactly one
relationship.
@return {Array} An array containing the internal models that `t` belongs
to or has many.
*/
_directlyRelatedInternalModels() {
let array = [];
this.type.eachRelationship((key, relationship) => {
if (this._relationships.has(key)) {
let related = this._relationships.get(key).members.toArray();
for (let i=0; i<related.length; ++i) {
array.push(related[i]);
}
}
});
return array;
}


/**
Computes the set of internal models reachable from this internaodel.
Reachability is determind over the relatinoship graph (ie a grawhere
nodes are internal models and edges are belongs to or has many
relationships.
@return {Array} An array including `this` and all internal modereachable
from `this`.
*/
_allRelatedInternalModels() {
let array = [];
let queue = [];
let bfsId = nextBfsId++;
queue.push(this);
this._bfsId = bfsId;
while (queue.length > 0) {
let node = queue.shift();
array.push(node);
let related = node._directlyRelatedInternalModels();
for (let i=0; i<related.length; ++i) {
let internalModel = related[i];
if (internalModel._bfsId < bfsId) {
queue.push(internalModel);
internalModel._bfsId = bfsId;
}
}
}
return array;
}


/**
Unload the record for this internal model. This will cause thecord to be
destroyed and freed up for garbage collection. It will also trer a check
for cleaning up internal models.
This check is performed by first computing the set of related irnal
models. If all records in this set are unloaded, then the entiset is
destroyed. Otherwise, nothing in the set is destroyed.
This means that this internal model will be freed up for garbagollection
once all models that refer to it via some relationship are alsoloaded.
*/
unloadRecord() {
this.send('unloadRecord');
this.dematerializeRecord();

let relatedInternalModels = this._allRelatedInternalModels();
if (areAllModelsUnloaded(relatedInternalModels)) {
for (let i=0; i<relatedInternalModels.length; ++i) {
let internalModel = relatedInternalModels[i];
if (!internalModel.isDestroying) {
internalModel.destroy();
}
}
}
}

eachRelationship(callback, binding) {
return this.type.eachRelationship(callback, binding);
}

destroy() {
this._isDestroying = true;
Ember.run.schedule('destroy', this, this.willDestroy, this);
}

willDestroy() {
assert("Cannot destroy an internalModel while its record materialized", !this.record);
this.store._removeFromIdMap(this);
this._isDestroyed = true;
}

eachAttribute(callback, binding) {
return this.type.eachAttribute(callback, binding);
}
Expand Down Expand Up @@ -375,11 +489,8 @@ export default class InternalModel {
return this._isDestroyed;
}

destroy() {
this._isDestroyed = true;
if (this.record) {
return this.record.destroy();
}
get isDestroying() {
return this._isDestroying;
}

/*
Expand Down
6 changes: 3 additions & 3 deletions addon/-private/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -808,9 +808,9 @@ var Model = Ember.Object.extend(Ember.Evented, {
willDestroy() {
//TODO Move!
this._super(...arguments);
this._internalModel.clearRelationships();
this._internalModel.recordObjectWillDestroy();
//TODO should we set internalModel to null here?

this._internalModel.resetRecord();
this._internalModel = null;
},

// This is a temporary solution until we refactor DS.Model to not
Expand Down
10 changes: 0 additions & 10 deletions addon/-private/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,6 @@ const RootState = {
// you out of the in-flight state.
rolledBack() { },
unloadRecord(internalModel) {
// clear relationships before moving to deleted state
// otherwise it fails
internalModel.clearRelationships();
internalModel.transitionTo('deleted.saved');
},

propertyWasReset() { },
Expand Down Expand Up @@ -573,10 +569,6 @@ const RootState = {
},

unloadRecord(internalModel) {
// clear relationships before moving to deleted state
// otherwise it fails
internalModel.clearRelationships();
internalModel.transitionTo('deleted.saved');
},

didCommit() {},
Expand Down Expand Up @@ -680,8 +672,6 @@ const RootState = {

setup(internalModel) {
internalModel.clearRelationships();
var store = internalModel.store;
store._dematerializeRecord(internalModel);
},

invokeLifecycleCallbacks(internalModel) {
Expand Down
5 changes: 2 additions & 3 deletions addon/-private/system/record-array-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ export default Ember.Object.extend({
updateRecordArrays() {
heimdall.increment(updateRecordArrays);
this.changedRecords.forEach(internalModel => {

if (internalModel.isDestroyed ||
internalModel.currentState.stateName === 'root.deleted.saved') {
if (internalModel.isDestroying ||
internalModel.currentState.stateName === 'root.deleted.saved') {
this._recordWasDeleted(internalModel);
} else {
this._recordWasChanged(internalModel);
Expand Down
21 changes: 5 additions & 16 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -1600,20 +1600,19 @@ Store = Service.extend({
let types = new Array(keys.length);

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

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

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

typeMap.metadata = new EmptyObject();
Expand Down Expand Up @@ -2441,21 +2440,11 @@ Store = Service.extend({
// . DESTRUCTION .
// ...............

/**
When a record is destroyed, this un-indexes it and
removes it from any record arrays so it can be GCed.
@method _dematerializeRecord
@private
@param {InternalModel} internalModel
*/
_dematerializeRecord(internalModel) {
_removeFromIdMap(internalModel) {
var type = internalModel.type;
var typeMap = this.typeMapFor(type);
var id = internalModel.id;

internalModel.updateRecordArrays();

if (id) {
delete typeMap.idToRecord[id];
}
Expand Down
9 changes: 2 additions & 7 deletions tests/integration/records/load-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,14 @@ module("integration/load - Loading Records", {
}
});

test("When loading a record fails, the isLoading is set to false", function(assert) {
test("When loading a record fails, the record is not left behind", function(assert) {
env.adapter.findRecord = function(store, type, id, snapshot) {
return Ember.RSVP.reject();
};

run(function() {
env.store.findRecord('post', 1).then(null, assert.wait(function() {
// store.recordForId is private, but there is currently no other way to
// get the specific record instance, since it is not passed to this
// rejection handler
var post = env.store.recordForId('post', 1);

assert.equal(post.get("isLoading"), false, "post is not loading anymore");
assert.equal(env.store.hasRecordForId('post', 1), false);
}));
});
});
Loading

0 comments on commit 484909c

Please sign in to comment.