perf: reduce notifications during first-load (#9673)

* perf: reduce notifications during first-load

* perf: silence more notifications

* cleanup impl

Author: runspired

packages/build-config/src/debugging.ts

@@ -103,3 +103,11 @@ export const LOG_GRAPH: boolean = false;
  * @public
  */
 export const LOG_INSTANCE_CACHE: boolean = false;
+/**
+ * Log key count metrics, useful for performance
+ * debugging.
+ *
+ * @property {boolean} LOG_METRIC_COUNTS
+ * @public
+ */
+export const LOG_METRIC_COUNTS: boolean = false;

packages/graph/src/-private/-diff.ts

@@ -298,10 +298,6 @@ export function _addLocal(
       relationship.localState.push(value);
     }
   }
-  assert(
-    `Expected relationship to be dirty when adding a local mutation`,
-    relationship.localState || relationship.isDirty
-  );
 
   return true;
 }

packages/graph/src/-private/-utils.ts

@@ -143,7 +143,7 @@ export function removeIdentifierCompletelyFromRelationship(
       // we shouldn't be notifying here though, figure out where
       // a notification was missed elsewhere.
       if (!silenceNotifications) {
-        notifyChange(graph, relationship.identifier, relationship.definition.key);
+        notifyChange(graph, relationship);
       }
     }
   } else if (isHasMany(relationship)) {
@@ -164,7 +164,7 @@ export function removeIdentifierCompletelyFromRelationship(
         // we shouldn't be notifying here though, figure out where
         // a notification was missed elsewhere.
         if (!silenceNotifications) {
-          notifyChange(graph, relationship.identifier, relationship.definition.key);
+          notifyChange(graph, relationship);
         }
       }
     }
@@ -174,8 +174,14 @@ export function removeIdentifierCompletelyFromRelationship(
   }
 }
 
-// TODO add silencing at the graph level
-export function notifyChange(graph: Graph, identifier: StableRecordIdentifier, key: string) {
+export function notifyChange(graph: Graph, relationship: CollectionEdge | ResourceEdge): void {
+  if (!relationship.accessed) {
+    return;
+  }
+
+  const identifier = relationship.identifier;
+  const key = relationship.definition.key;
+
   if (identifier === graph._removing) {
     if (LOG_GRAPH) {
       // eslint-disable-next-line no-console

packages/graph/src/-private/edges/collection.ts

@@ -22,8 +22,24 @@ export interface CollectionEdge {
   links: Links | PaginationLinks | null;
 
   localState: StableRecordIdentifier[] | null;
+  /**
+   * Whether the localState for this edge is out-of-sync
+   * with the remoteState.
+   *
+   * if state.hasReceivedData=false we are also
+   * not dirty since there is nothing to sync with.
+   *
+   * @typedoc
+   */
   isDirty: boolean;
   transactionRef: number;
+  /**
+   * Whether data for this edge has been accessed at least once
+   * via `graph.getData`
+   *
+   * @typedoc
+   */
+  accessed: boolean;
 
   _diff?: {
     add: Set<StableRecordIdentifier>;
@@ -45,13 +61,15 @@ export function createCollectionEdge(definition: UpgradedMeta, identifier: Stabl
     links: null,
 
     localState: null,
-    isDirty: true,
+    isDirty: false,
     transactionRef: 0,
+    accessed: false,
     _diff: undefined,
   };
 }
 
 export function legacyGetCollectionRelationshipData(source: CollectionEdge): CollectionRelationship {
+  source.accessed = true;
   const payload: CollectionRelationship = {};
 
   if (source.state.hasReceivedData) {

packages/graph/src/-private/edges/resource.ts

@@ -23,6 +23,7 @@ export interface ResourceEdge {
   meta: Meta | null;
   links: Links | PaginationLinks | null;
   transactionRef: number;
+  accessed: boolean;
 }
 
 export function createResourceEdge(definition: UpgradedMeta, identifier: StableRecordIdentifier): ResourceEdge {
@@ -35,10 +36,12 @@ export function createResourceEdge(definition: UpgradedMeta, identifier: StableR
     remoteState: null,
     meta: null,
     links: null,
+    accessed: false,
   };
 }
 
 export function legacyGetResourceRelationshipData(source: ResourceEdge): ResourceRelationship {
+  source.accessed = true;
   let data: StableRecordIdentifier | null | undefined;
   const payload: ResourceRelationship = {};
   if (source.localState) {

packages/graph/src/-private/graph.ts

@@ -559,7 +559,7 @@ export class Graph {
     this._willSyncLocal = false;
     const updated = this._updatedRelationships;
     this._updatedRelationships = new Set();
-    updated.forEach((rel) => notifyChange(this, rel.identifier, rel.definition.key));
+    updated.forEach((rel) => notifyChange(this, rel));
   }
 
   destroy() {
@@ -641,7 +641,7 @@ function destroyRelationship(graph: Graph, rel: GraphEdge, silenceNotifications?
     // leave the ui relationship populated since the record is destroyed and
     // internally we've fully cleaned up.
     if (!rel.definition.isAsync && !silenceNotifications) {
-      /*#__NOINLINE__*/ notifyChange(graph, rel.identifier, rel.definition.key);
+      /*#__NOINLINE__*/ notifyChange(graph, rel);
     }
   }
 }
@@ -713,7 +713,7 @@ function removeDematerializedInverse(
     }
 
     if (!silenceNotifications) {
-      notifyChange(graph, relationship.identifier, relationship.definition.key);
+      notifyChange(graph, relationship);
     }
   } else {
     if (!relationship.definition.isAsync || (inverseIdentifier && isNew(inverseIdentifier))) {
@@ -728,7 +728,7 @@ function removeDematerializedInverse(
     }
 
     if (!silenceNotifications) {
-      notifyChange(graph, relationship.identifier, relationship.definition.key);
+      notifyChange(graph, relationship);
     }
   }
 }
@@ -753,7 +753,7 @@ function removeCompletelyFromInverse(graph: Graph, relationship: GraphEdge) {
     if (!relationship.definition.isAsync) {
       clearRelationship(relationship);
 
-      notifyChange(graph, relationship.identifier, relationship.definition.key);
+      notifyChange(graph, relationship);
     }
   } else {
     relationship.remoteMembers.clear();

packages/graph/src/-private/operations/add-to-related-records.ts

@@ -19,6 +19,13 @@ export default function addToRelatedRecords(graph: Graph, op: AddToRelatedRecord
     `You can only '${op.op}' on a hasMany relationship. ${record.type}.${op.field} is a ${relationship.definition.kind}`,
     isHasMany(relationship)
   );
+
+  // if we are not dirty but have a null localState then we
+  // are mutating a relationship that has never been fetched
+  // so we initialize localState to an empty array
+  if (!relationship.isDirty && !relationship.localState) {
+    relationship.localState = [];
+  }
   if (Array.isArray(value)) {
     for (let i = 0; i < value.length; i++) {
       addRelatedRecord(graph, relationship, record, value[i], index !== undefined ? index + i : index, isRemote);
@@ -27,7 +34,7 @@ export default function addToRelatedRecords(graph: Graph, op: AddToRelatedRecord
     addRelatedRecord(graph, relationship, record, value, index, isRemote);
   }
 
-  notifyChange(graph, relationship.identifier, relationship.definition.key);
+  notifyChange(graph, relationship);
 }
 
 function addRelatedRecord(

packages/graph/src/-private/operations/merge-identifier.ts

@@ -40,7 +40,7 @@ function mergeBelongsTo(graph: Graph, rel: ResourceEdge, op: MergeOperation): vo
   }
   if (rel.localState === op.record) {
     rel.localState = op.value;
-    notifyChange(graph, rel.identifier, rel.definition.key);
+    notifyChange(graph, rel);
   }
 }
 
@@ -63,7 +63,7 @@ function mergeHasMany(graph: Graph, rel: CollectionEdge, op: MergeOperation): vo
     rel.isDirty = true;
   }
   if (rel.isDirty) {
-    notifyChange(graph, rel.identifier, rel.definition.key);
+    notifyChange(graph, rel);
   }
 }
 

packages/graph/src/-private/operations/remove-from-related-records.ts

@@ -30,7 +30,7 @@ export default function removeFromRelatedRecords(graph: Graph, op: RemoveFromRel
   } else {
     removeRelatedRecord(graph, relationship, record, value, isRemote);
   }
-  notifyChange(graph, relationship.identifier, relationship.definition.key);
+  notifyChange(graph, relationship);
 }
 
 function removeRelatedRecord(

packages/graph/src/-private/operations/replace-related-record.ts

@@ -107,7 +107,7 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec
             }
           );
 
-          notifyChange(graph, relationship.identifier, relationship.definition.key);
+          notifyChange(graph, relationship);
         }
       }
     }
@@ -156,7 +156,7 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec
     // and we can safely sync the new remoteState to local
     if (localState !== remoteState && localState === existingState) {
       relationship.localState = remoteState;
-      notifyChange(graph, relationship.identifier, relationship.definition.key);
+      notifyChange(graph, relationship);
       // But when localState does not match the new remoteState and
       // and localState !== existingState then we know we have a local mutation
       // that has not been persisted yet.
@@ -186,10 +186,10 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec
           }
         );
 
-        notifyChange(graph, relationship.identifier, relationship.definition.key);
+        notifyChange(graph, relationship);
       }
     }
   } else {
-    notifyChange(graph, relationship.identifier, relationship.definition.key);
+    notifyChange(graph, relationship);
   }
 }

packages/graph/src/-private/operations/replace-related-records.ts

@@ -80,12 +80,6 @@ function replaceRelatedRecordsLocal(graph: Graph, op: ReplaceRelatedRecordsOpera
   const relationship = graph.get(op.record, op.field);
   assert(`expected hasMany relationship`, isHasMany(relationship));
 
-  // relationships for newly created records begin in the dirty state, so if updated
-  // before flushed we would fail to notify. This check helps us avoid that.
-  const isMaybeFirstUpdate =
-    relationship.remoteState.length === 0 &&
-    relationship.localState === null &&
-    relationship.state.hasReceivedData === false;
   relationship.state.hasReceivedData = true;
   const { additions, removals } = relationship;
   const { inverseKey, type } = relationship.definition;
@@ -158,13 +152,11 @@ function replaceRelatedRecordsLocal(graph: Graph, op: ReplaceRelatedRecordsOpera
   relationship.additions = diff.add;
   relationship.removals = diff.del;
   relationship.localState = diff.finalState;
-  relationship.isDirty = wasDirty;
 
-  // we notify if this is the first update to the relationship
-  // because ?? may need to recalculate.
-  // otherwise we only notify if we are dirty and were not already dirty before
-  if (isMaybeFirstUpdate || (becameDirty && !wasDirty)) {
-    notifyChange(graph, op.record, op.field);
+  // we only notify if the localState changed and were not already dirty before
+  // because if we were already dirty then we have already notified
+  if (becameDirty && !wasDirty) {
+    notifyChange(graph, relationship);
   }
 }
 
@@ -180,8 +172,14 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper
     graph._addToTransaction(relationship);
   }
 
-  // see note before flushCanonical
-  // const wasDirty = relationship.isDirty;
+  const wasDirty = relationship.isDirty;
+  // if this is our first time receiving data
+  // we need to mark the relationship as dirty
+  // so that non-materializing APIs like `hasManyReference.value()`
+  // will get notified and updated.
+  if (!relationship.state.hasReceivedData) {
+    relationship.isDirty = true;
+  }
   relationship.state.hasReceivedData = true;
 
   // cache existing state
@@ -310,10 +308,7 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper
     }
   }
 
-  // we ought to only flush if we became dirty and were not before
-  // but this causes a fw test failures around unloadRecord and reference autotracking
-  // we should investigate this further
-  if (relationship.isDirty /*&& !wasDirty*/) {
+  if (relationship.isDirty && !wasDirty) {
     flushCanonical(graph, relationship);
   }
 }
@@ -352,7 +347,8 @@ export function addToInverse(
         removeFromInverse(graph, relationship.localState, relationship.definition.inverseKey, identifier, isRemote);
       }
       relationship.localState = value;
-      notifyChange(graph, identifier, key);
+
+      notifyChange(graph, relationship);
     }
   } else if (isHasMany(relationship)) {
     if (isRemote) {
@@ -374,8 +370,15 @@ export function addToInverse(
         }
       }
     } else {
+      // if we are not dirty but have a null localState then we
+      // are mutating a relationship that has never been fetched
+      // so we initialize localState to an empty array
+      if (!relationship.isDirty && !relationship.localState) {
+        relationship.localState = [];
+      }
+
       if (_addLocal(graph, identifier, relationship, value, null)) {
-        notifyChange(graph, identifier, key);
+        notifyChange(graph, relationship);
       }
     }
   } else {
@@ -401,7 +404,7 @@ export function notifyInverseOfPotentialMaterialization(
 ) {
   const relationship = graph.get(identifier, key);
   if (isHasMany(relationship) && isRemote && relationship.remoteMembers.has(value)) {
-    notifyChange(graph, identifier, key);
+    notifyChange(graph, relationship);
   }
 }
 
@@ -423,17 +426,17 @@ export function removeFromInverse(
     if (relationship.localState === value) {
       relationship.localState = null;
 
-      notifyChange(graph, identifier, key);
+      notifyChange(graph, relationship);
     }
   } else if (isHasMany(relationship)) {
     if (isRemote) {
       graph._addToTransaction(relationship);
       if (_removeRemote(relationship, value)) {
-        notifyChange(graph, identifier, key);
+        notifyChange(graph, relationship);
       }
     } else {
       if (_removeLocal(relationship, value)) {
-        notifyChange(graph, identifier, key);
+        notifyChange(graph, relationship);
       }
     }
   } else {
@@ -449,5 +452,7 @@ export function removeFromInverse(
 }
 
 function flushCanonical(graph: Graph, rel: CollectionEdge) {
-  graph._scheduleLocalSync(rel);
+  if (rel.accessed) {
+    graph._scheduleLocalSync(rel);
+  }
 }

packages/graph/src/-private/operations/update-relationship.ts

@@ -157,7 +157,7 @@ export default function updateRelationshipOperation(graph: Graph, op: UpdateRela
     ) {
       relationship.state.isStale = true;
 
-      notifyChange(graph, relationship.identifier, relationship.definition.key);
+      notifyChange(graph, relationship);
     } else {
       relationship.state.isStale = false;
     }