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.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
  • Loading branch information
hjdivad committed Jan 16, 2017
1 parent ae4aa84 commit b387340
Show file tree
Hide file tree
Showing 21 changed files with 647 additions and 184 deletions.
183 changes: 166 additions & 17 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(internalModels) {
for (let i=0; i<internalModels.length; ++i) {
let record = internalModels[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 @@ -88,6 +98,7 @@ const {
);

let InternalModelReferenceId = 1;
let nextBfsId = 1;

/*
`InternalModel` is the Model class that we use internally inside Ember Data to represent models.
Expand All @@ -111,28 +122,38 @@ export default class InternalModel {
this.id = id;
this._internalId = InternalModelReferenceId++;
this.store = store;
this._data = data || new EmptyObject();
this.modelName = modelName;
this.dataHasInitialized = false;
this._loadingPromise = null;
this._record = null;
this.currentState = RootState.empty;
this.isReloading = false;
this._isDestroyed = false;
this.isError = false;
this.error = null;
this._isUpdatingRecordArrays = false;

// During dematerialization we don't want to rematerialize the record. The
// reason this might happen is that dematerialization removes records from
// record arrays, and Ember arrays will always `objectAt(0)` and
// `objectAt(len - 1)` to test whether or not `firstObject` or `lastObject`
// have changed.
this._isDematerializing = false;

this.resetRecord();

if (data) {
this.__data = data;
}

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

// Used during the mark phase of unloading to avoid checking the same internal
// model twice in the same scan
this._bfsId = 0;
}

get modelClass() {
Expand All @@ -145,7 +166,7 @@ export default class InternalModel {

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 @@ -201,6 +222,17 @@ export default class InternalModel {
this.__inFlightAttributes = v;
}

get _data() {
if (this.__data === null) {
this.__data = new EmptyObject();
}
return this.__data;
}

set _data(v) {
this.__data = v;
}

/*
implicit relationships are relationship which have not been declared but the inverse side exists on
another record somewhere
Expand Down Expand Up @@ -276,7 +308,7 @@ export default class InternalModel {
}

getRecord() {
if (!this._record) {
if (!this._record && !this._isDematerializing) {
heimdall.increment(materializeRecord);
let token = heimdall.start('InternalModel.getRecord');

Expand Down Expand Up @@ -307,8 +339,25 @@ export default class InternalModel {
return this._record;
}

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

dematerializeRecord() {
if (this.record) {
this._isDematerializing = true;
this.record.destroy();
this.destroyRelationships();
this.updateRecordArrays();
this.resetRecord();
}
}

deleteRecord() {
Expand Down Expand Up @@ -356,14 +405,109 @@ export default class InternalModel {
});
}

/**
Computes the set of internal models reachable from `this` across exactly one
relationship.
@return {Array} An array containing the internal models that `this` belongs
to or has many.
*/
_directlyRelatedInternalModels() {
let array = [];
this.type.eachRelationship((key, relationship) => {
if (this._relationships.has(key)) {
let relationship = this._relationships.get(key);
let localRelationships = relationship.members.toArray();
let serverRelationships = relationship.canonicalMembers.toArray();

array = array.concat(localRelationships, serverRelationships);
}
});
return array;
}


/**
Computes the set of internal models reachable from this internal model.
Reachability is determined over the relationship graph (ie a graph where
nodes are internal models and edges are belongs to or has many
relationships).
@return {Array} An array including `this` and all internal models reachable
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];
assert('Internal Error: seen a future bfs iteration', internalModel._bfsId <= bfsId);
if (internalModel._bfsId < bfsId) {
queue.push(internalModel);
internalModel._bfsId = bfsId;
}
}
}
return array;
}


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

_checkForOrphanedInternalModels() {
this._isDematerializing = false;
if (this.isDestroyed) { return; }

this._cleanupOrphanedInternalModels();
}

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

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

destroy() {
assert("Cannot destroy an internalModel while its record is materialized", !this.record || this.record.get('isDestroyed') || this.record.get('isDestroying'));

this.store._removeFromIdMap(this);
this._isDestroyed = true;
}

eachAttribute(callback, binding) {
return this.modelClass.eachAttribute(callback, binding);
}
Expand Down Expand Up @@ -402,13 +546,6 @@ export default class InternalModel {
return !!this._record;
}

destroy() {
this._isDestroyed = true;
if (this.hasRecord) {
return this.record.destroy();
}
}

/*
@method createSnapshot
@private
Expand Down Expand Up @@ -718,6 +855,18 @@ export default class InternalModel {
});
}

destroyRelationships() {
this.eachRelationship((name, relationship) => {
if (this._relationships.has(name)) {
let rel = this._relationships.get(name);
rel.destroy();
}
});
Object.keys(this._implicitRelationships).forEach((key) => {
this._implicitRelationships[key].destroy();
});
}

/*
When a find request is triggered on the store, the user can optionally pass in
attributes and relationships to be preloaded. These are meant to behave as if they
Expand Down
8 changes: 0 additions & 8 deletions addon/-private/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -807,14 +807,6 @@ const Model = Ember.Object.extend(Ember.Evented, {
this._super(...arguments);
},

willDestroy() {
//TODO Move!
this._super(...arguments);
this._internalModel.clearRelationships();
this._internalModel.recordObjectWillDestroy();
//TODO should we set internalModel to null here?
},

// This is a temporary solution until we refactor DS.Model to not
// rely on the data property.
willMergeMixin(props) {
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
11 changes: 9 additions & 2 deletions addon/-private/system/record-array-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,15 @@ export default class RecordArrayManager {
for (let i = 0, l = updated.length; i < l; i++) {
let internalModel = updated[i];

if (internalModel.isDestroyed ||
internalModel.currentState.stateName === 'root.deleted.saved') {
// During dematerialization we don't want to rematerialize the record.
// recordWasDeleted can cause other records to rematerialize because it
// removes the internal model from the array and Ember arrays will always
// `objectAt(0)` and `objectAt(len -1)` to check whether `firstObject` or
// `lastObject` have changed. When this happens we don't want those
// models to rematerialize their records.
if (internalModel._isDematerializing ||
internalModel.isDestroyed ||
internalModel.currentState.stateName === 'root.deleted.saved') {
this._recordWasDeleted(internalModel);
} else {
this._recordWasChanged(internalModel);
Expand Down
1 change: 0 additions & 1 deletion addon/-private/system/record-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ export default class RecordMap {
for (let i = 0; i < records.length; i++) {
record = records[i];
record.unloadRecord();
record.destroy(); // maybe within unloadRecord
}
}

Expand Down
12 changes: 10 additions & 2 deletions addon/-private/system/relationships/state/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ export default class BelongsToRelationship extends Relationship {
super.addCanonicalRecord(newRecord);
}

inverseDidDematerialize() {
this.notifyBelongsToChanged();
}

flushCanonical() {
//temporary fix to not remove newly created records if server returned null.
//TODO remove once we have proper diffing
Expand All @@ -54,7 +58,7 @@ export default class BelongsToRelationship extends Relationship {
}
if (this.inverseRecord !== this.canonicalState) {
this.inverseRecord = this.canonicalState;
this.internalModel.notifyBelongsToChanged(this.key);
this.notifyBelongsToChanged();
}

super.flushCanonical();
Expand All @@ -71,7 +75,7 @@ export default class BelongsToRelationship extends Relationship {

this.inverseRecord = newRecord;
super.addRecord(newRecord);
this.internalModel.notifyBelongsToChanged(this.key);
this.notifyBelongsToChanged();
}

setRecordPromise(newPromise) {
Expand All @@ -84,6 +88,10 @@ export default class BelongsToRelationship extends Relationship {
if (!this.members.has(record)) { return;}
this.inverseRecord = null;
super.removeRecordFromOwn(record);
this.notifyBelongsToChanged();
}

notifyBelongsToChanged() {
this.internalModel.notifyBelongsToChanged(this.key);
}

Expand Down
Loading

0 comments on commit b387340

Please sign in to comment.