Skip to content

Commit

Permalink
perf: silence more notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
runspired committed Feb 10, 2025
1 parent b390ccc commit 90bb8b2
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 29 deletions.
14 changes: 10 additions & 4 deletions packages/graph/src/-private/-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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);
}
}
}
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/graph/src/-private/edges/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface CollectionEdge {
localState: StableRecordIdentifier[] | null;
isDirty: boolean;
transactionRef: number;
accessed: boolean;

_diff?: {
add: Set<StableRecordIdentifier>;
Expand All @@ -47,11 +48,13 @@ export function createCollectionEdge(definition: UpgradedMeta, identifier: Stabl
localState: null,
isDirty: true,
transactionRef: 0,
accessed: false,
_diff: undefined,
};
}

export function legacyGetCollectionRelationshipData(source: CollectionEdge): CollectionRelationship {
source.accessed = true;
const payload: CollectionRelationship = {};

if (source.state.hasReceivedData) {
Expand Down
3 changes: 3 additions & 0 deletions packages/graph/src/-private/edges/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions packages/graph/src/-private/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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))) {
Expand All @@ -728,7 +728,7 @@ function removeDematerializedInverse(
}

if (!silenceNotifications) {
notifyChange(graph, relationship.identifier, relationship.definition.key);
notifyChange(graph, relationship);
}
}
}
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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(
Expand Down
4 changes: 2 additions & 2 deletions packages/graph/src/-private/operations/merge-identifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export default function replaceRelatedRecord(graph: Graph, op: ReplaceRelatedRec
}
);

notifyChange(graph, relationship.identifier, relationship.definition.key);
notifyChange(graph, relationship);
}
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}
20 changes: 9 additions & 11 deletions packages/graph/src/-private/operations/replace-related-records.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ function replaceRelatedRecordsLocal(graph: Graph, op: ReplaceRelatedRecordsOpera
// 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);
notifyChange(graph, relationship);
}
}

Expand Down Expand Up @@ -352,7 +352,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) {
Expand All @@ -375,7 +376,7 @@ export function addToInverse(
}
} else {
if (_addLocal(graph, identifier, relationship, value, null)) {
notifyChange(graph, identifier, key);
notifyChange(graph, relationship);
}
}
} else {
Expand All @@ -401,7 +402,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);
}
}

Expand All @@ -423,17 +424,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 {
Expand All @@ -449,10 +450,7 @@ export function removeFromInverse(
}

function flushCanonical(graph: Graph, rel: CollectionEdge) {
// if this relationship does not have localState then
// we have never computed it before, meaning it has no
// possible subscribers.
if (rel.localState !== null) {
if (rel.accessed) {
graph._scheduleLocalSync(rel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 90bb8b2

Please sign in to comment.