[PERF] Class Fields Use Optimization & Made OrderedSet Faster (Again)

packages/-ember-data/package.json

@@ -30,7 +30,6 @@
     "@ember-data/serializer": "3.28.0-alpha.0",
     "@ember-data/store": "3.28.0-alpha.0",
     "@ember/edition-utils": "^1.2.0",
-    "@ember/ordered-set": "^4.0.0",
     "@ember/string": "^1.0.0",
     "@glimmer/env": "^0.1.7",
     "broccoli-merge-trees": "^4.2.0",

packages/record-data/addon/-private/ordered-set.ts

@@ -1,22 +1,118 @@
 import { assert } from '@ember/debug';
-import { guidFor } from '@ember/object/internals';
-import EmberOrderedSet from '@ember/ordered-set';
 
-export default class EmberDataOrderedSet<T> extends EmberOrderedSet<T> {
-  static create() {
-    return new this();
+type Dict<T> = import('@ember-data/store/-private/ts-interfaces/utils').Dict<T>;
+type RelationshipRecordData = import('./ts-interfaces/relationship-record-data').RelationshipRecordData;
+
+const NULL_POINTER = `null-${Date.now()}`;
+
+/*
+ TODO Ember's guidFor returns a new string per-object reference
+ while ours does not.
+
+ This has surfaced a bug during resurrection
+ in which Ember's guidFor would return false for `has` since the
+ resurrected record receives a fresh RecordData instance, leaving
+ the destroyed record in the set and thus depending on the state flags
+ for it not appearing elsewhere. We've accounted for this bug in the
+ updated OrderedSet implementation by doing a reference check: e.g.
+ the bug is preserved.
+
+ While we convert relationships to identifiers this will be something we
+ will be forced to address.
+*/
+function guidFor(obj): string {
+  if (obj === null) {
+    return NULL_POINTER;
+  }
+
+  return obj.clientId || obj.lid;
+}
+
+export default class OrderedSet {
+  declare presenceSet: Dict<RelationshipRecordData | null>;
+  declare list: (RelationshipRecordData | null)[];
+  declare size: number;
+
+  constructor() {
+    this.clear();
+  }
+
+  clear() {
+    this.presenceSet = Object.create(null);
+    this.list = [];
+    this.size = 0;
+  }
+
+  add(obj: RelationshipRecordData | null): OrderedSet {
+    let guid = guidFor(obj);
+    assert(`Expected ${obj} to have an clientId`, typeof guid === 'string' && guid !== '');
+    let presenceSet = this.presenceSet;
+    let list = this.list;
+
+    if (presenceSet[guid] !== obj) {
+      presenceSet[guid] = obj;
+      this.size = list.push(obj);
+    }
+
+    return this;
+  }
+
+  delete(obj: RelationshipRecordData | null): boolean {
+    let guid = guidFor(obj);
+    assert(`Expected ${obj} to have an clientId`, typeof guid === 'string' && guid !== '');
+    let presenceSet = this.presenceSet;
+    let list = this.list;
+
+    if (presenceSet[guid] === obj) {
+      delete presenceSet[guid];
+      let index = list.indexOf(obj);
+      if (index > -1) {
+        list.splice(index, 1);
+      }
+      this.size = list.length;
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  has(obj: RelationshipRecordData | null): boolean {
+    if (this.size === 0) {
+      return false;
+    }
+    let guid = guidFor(obj);
+    assert(`Expected ${obj} to have an clientId`, typeof guid === 'string' && guid !== '');
+    return this.presenceSet[guid] === obj;
+  }
+
+  toArray(): (RelationshipRecordData | null)[] {
+    return this.list.slice();
+  }
+
+  copy(): OrderedSet {
+    let set = new OrderedSet();
+
+    for (let prop in this.presenceSet) {
+      set.presenceSet[prop] = this.presenceSet[prop];
+    }
+
+    set.list = this.toArray();
+    set.size = this.size;
+
+    return set;
   }
 
-  addWithIndex(obj: T, idx?: number) {
+  addWithIndex(obj: RelationshipRecordData | null, idx?: number) {
     let guid = guidFor(obj);
+    assert(`Expected ${obj} to have an clientId`, typeof guid === 'string' && guid !== '');
     let presenceSet = this.presenceSet;
     let list = this.list;
 
-    if (presenceSet[guid] === true) {
+    if (presenceSet[guid] === obj) {
       return;
     }
 
-    presenceSet[guid] = true;
+    presenceSet[guid] = obj;
 
     if (idx === undefined || idx === null) {
       list.push(obj);
@@ -29,12 +125,13 @@ export default class EmberDataOrderedSet<T> extends EmberOrderedSet<T> {
     return this;
   }
 
-  deleteWithIndex(obj: T | null, idx?: number): boolean {
+  deleteWithIndex(obj: RelationshipRecordData | null, idx?: number): boolean {
     let guid = guidFor(obj);
+    assert(`Expected ${obj} to have an clientId`, typeof guid === 'string' && guid !== '');
     let presenceSet = this.presenceSet;
     let list = this.list;
 
-    if (presenceSet[guid] === true) {
+    if (presenceSet[guid] === obj) {
       delete presenceSet[guid];
 
       assert('object is not present at specified index', idx === undefined || list[idx] === obj);

packages/record-data/addon/-private/relationships/state/relationship.ts

@@ -27,8 +27,8 @@ export default class Relationship {
   inverseIsAsync: boolean | undefined;
   kind?: string;
   recordData: RelationshipRecordData;
-  members: OrderedSet<RelationshipRecordData>;
-  canonicalMembers: OrderedSet<RelationshipRecordData>;
+  members: OrderedSet;
+  canonicalMembers: OrderedSet;
   store: any;
   key: string | null;
   inverseKey: string | null;
@@ -235,7 +235,7 @@ export default class Relationship {
     // we actually want a union of members and canonicalMembers
     // they should be disjoint but currently are not due to a bug
     this.forAllMembers(inverseRecordData => {
-      if (!this._hasSupportForRelationships(inverseRecordData)) {
+      if (!inverseRecordData || !this._hasSupportForRelationships(inverseRecordData)) {
         return;
       }
       let relationship = relationshipStateFor(inverseRecordData, inverseKey);
@@ -253,7 +253,7 @@ export default class Relationship {
     });
   }
 
-  forAllMembers(callback: (im: RelationshipRecordData) => void) {
+  forAllMembers(callback: (im: RelationshipRecordData | null) => void) {
     let seen = Object.create(null);
 
     for (let i = 0; i < this.members.list.length; i++) {
@@ -380,13 +380,14 @@ export default class Relationship {
     }
   }
 
-  removeCanonicalRecordData(recordData: RelationshipRecordData, idx?: number) {
+  removeCanonicalRecordData(recordData: RelationshipRecordData | null, idx?: number) {
     if (this.canonicalMembers.has(recordData)) {
       this.removeCanonicalRecordDataFromOwn(recordData, idx);
       if (this.inverseKey) {
         this.removeCanonicalRecordDataFromInverse(recordData);
       } else {
         if (
+          recordData !== null &&
           this._hasSupportForImplicitRelationships(recordData) &&
           recordData._implicitRelationships[this.inverseKeyForImplicit]
         ) {
@@ -421,13 +422,14 @@ export default class Relationship {
     this.setHasAnyRelationshipData(true);
   }
 
-  removeRecordData(recordData: RelationshipRecordData) {
+  removeRecordData(recordData: RelationshipRecordData | null) {
     if (this.members.has(recordData)) {
       this.removeRecordDataFromOwn(recordData);
       if (this.inverseKey) {
         this.removeRecordDataFromInverse(recordData);
       } else {
         if (
+          recordData !== null &&
           this._hasSupportForImplicitRelationships(recordData) &&
           recordData._implicitRelationships[this.inverseKeyForImplicit]
         ) {
@@ -437,8 +439,8 @@ export default class Relationship {
     }
   }
 
-  removeRecordDataFromInverse(recordData: RelationshipRecordData) {
-    if (!this._hasSupportForRelationships(recordData)) {
+  removeRecordDataFromInverse(recordData: RelationshipRecordData | null) {
+    if (!recordData || !this._hasSupportForRelationships(recordData)) {
       return;
     }
     if (this.inverseKey) {
@@ -454,8 +456,8 @@ export default class Relationship {
     this.members.delete(recordData);
   }
 
-  removeCanonicalRecordDataFromInverse(recordData: RelationshipRecordData) {
-    if (!this._hasSupportForRelationships(recordData)) {
+  removeCanonicalRecordDataFromInverse(recordData: RelationshipRecordData | null) {
+    if (!recordData || !this._hasSupportForRelationships(recordData)) {
       return;
     }
     if (this.inverseKey) {
@@ -515,8 +517,8 @@ export default class Relationship {
       };
     }
 
-    this.members.forEach(unload);
-    this.canonicalMembers.forEach(unload);
+    this.members.toArray().forEach(unload);
+    this.canonicalMembers.toArray().forEach(unload);
 
     if (!this.isAsync) {
       this.clear();

packages/record-data/index.js

@@ -12,7 +12,6 @@ module.exports = Object.assign({}, addonBaseConfig, {
     return [
       '@ember-data/store/-debug',
       '@ember-data/store/-private',
-      '@ember/ordered-set',
       '@ember-data/store',
       '@ember-data/canary-features',
     ];

packages/record-data/package.json

@@ -23,7 +23,6 @@
     "@ember-data/private-build-infra": "3.28.0-alpha.0",
     "@ember-data/store": "3.28.0-alpha.0",
     "@ember/edition-utils": "^1.2.0",
-    "@ember/ordered-set": "^4.0.0",
     "ember-cli-babel": "^7.26.3",
     "ember-cli-test-info": "^1.0.0",
     "ember-cli-typescript": "^4.0.0"