Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: reduce simple Map/Set ops #9761

Merged
merged 1 commit into from
Mar 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/build-config/src/debugging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,15 @@ export const LOG_METRIC_COUNTS: boolean = false;
* @public
*/
export const DEBUG_RELATIONSHIP_NOTIFICATIONS: boolean = false;

/**
* A private flag to enable logging of the native Map/Set
* constructor and method calls.
*
* EXTREMELY MALPERFORMANT
*
* LOG_METRIC_COUNTS must also be enabled.
*
* @typedoc
*/
export const __INTERNAL_LOG_NATIVE_MAP_SET_COUNTS: boolean = false;
23 changes: 13 additions & 10 deletions packages/graph/src/-private/graph.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is just variable name cleanup. This method is a mega hot-path. I previously benchmarked this multi-tier strategy working better overall than a single-tier strategy + there may later be gains made possible by it in terms of making remote operations flush lazily.

However, we should re-test this assumption at this point.

Original file line number Diff line number Diff line change
Expand Up @@ -781,19 +781,22 @@ function addPending(
definition: UpgradedMeta,
op: RemoteRelationshipOperation & { field: string }
): void {
const lc = (cache[definition.kind as 'hasMany' | 'belongsTo'] =
const cacheForKind = (cache[definition.kind as 'hasMany' | 'belongsTo'] =
cache[definition.kind as 'hasMany' | 'belongsTo'] || new Map<string, Map<string, RemoteRelationshipOperation[]>>());
let lc2 = lc.get(definition.inverseType);
if (!lc2) {
lc2 = new Map<string, RemoteRelationshipOperation[]>();
lc.set(definition.inverseType, lc2);

let cacheForType = cacheForKind.get(definition.inverseType);
if (!cacheForType) {
cacheForType = new Map<string, RemoteRelationshipOperation[]>();
cacheForKind.set(definition.inverseType, cacheForType);
}
let arr = lc2.get(op.field);
if (!arr) {
arr = [];
lc2.set(op.field, arr);

let cacheForField = cacheForType.get(op.field);
if (!cacheForField) {
cacheForField = [];
cacheForType.set(op.field, cacheForField);
}
arr.push(op);

cacheForField.push(op);
}

function isReordered(relationship: CollectionEdge): boolean {
Expand Down
4 changes: 2 additions & 2 deletions packages/json-api/src/-private/cache.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for small arrays and sets, checking value directly is 30% faster (or more)

Original file line number Diff line number Diff line change
Expand Up @@ -1992,9 +1992,9 @@ function setupRelationships(
}
}

const RelationshipKinds = new Set(['hasMany', 'belongsTo', 'resource', 'collection']);
function isRelationship(field: FieldSchema): field is LegacyRelationshipSchema | CollectionField | ResourceField {
return RelationshipKinds.has(field.kind);
const { kind } = field;
return kind === 'hasMany' || kind === 'belongsTo' || kind === 'resource' || kind === 'collection';
}

function patchLocalAttributes(cached: CachedResource, changedRemoteKeys?: Set<string>): boolean {
Expand Down
73 changes: 43 additions & 30 deletions packages/store/src/-private/managers/notification-manager.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change here is that we use the callback function itself as the token, stashing a bit of info directly on it. This eliminates the double map lookup and simplifies iteration.

We also convert CacheOperations (a simple set) into a direct value comparison.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import { _backburner } from '@ember/runloop';

import { LOG_METRIC_COUNTS, LOG_NOTIFICATIONS } from '@warp-drive/build-config/debugging';
import { DEBUG } from '@warp-drive/build-config/env';
import { assert } from '@warp-drive/build-config/macros';
import type { StableDocumentIdentifier, StableRecordIdentifier } from '@warp-drive/core-types/identifier';

Expand All @@ -14,14 +13,14 @@ import { log } from '../debug/utils';
import type { Store } from '../store-service';

export type UnsubscribeToken = object;
let tokenId = 0;

const CacheOperations = new Set(['added', 'removed', 'state', 'updated', 'invalidated']);
export type CacheOperation = 'added' | 'removed' | 'updated' | 'state';
export type DocumentCacheOperation = 'invalidated' | 'added' | 'removed' | 'updated' | 'state';

function isCacheOperationValue(value: NotificationType | DocumentCacheOperation): value is DocumentCacheOperation {
return CacheOperations.has(value);
return (
value === 'added' || value === 'state' || value === 'updated' || value === 'removed' || value === 'invalidated'
);
}

function runLoopIsFlushing(): boolean {
Expand Down Expand Up @@ -54,25 +53,40 @@ function count(label: string) {
globalThis.__WarpDriveMetricCountData[label] = (globalThis.__WarpDriveMetricCountData[label] || 0) + 1;
}

function asInternalToken(token: unknown): asserts token is {
for: StableDocumentIdentifier | StableRecordIdentifier | 'resource' | 'document';
} & (NotificationCallback | ResourceOperationCallback | DocumentOperationCallback) {
assert(`Expected a token with a 'for' property`, token && typeof token === 'function' && 'for' in token);
}

function _unsubscribe(
tokens: Map<UnsubscribeToken, StableDocumentIdentifier | StableRecordIdentifier | 'resource' | 'document'>,
token: UnsubscribeToken,
cache: Map<
'resource' | 'document' | StableDocumentIdentifier | StableRecordIdentifier,
Map<UnsubscribeToken, NotificationCallback | ResourceOperationCallback | DocumentOperationCallback>
Array<NotificationCallback | ResourceOperationCallback | DocumentOperationCallback>
>
) {
const identifier = tokens.get(token);
asInternalToken(token);
const identifier = token.for;
if (LOG_NOTIFICATIONS) {
if (!identifier) {
// eslint-disable-next-line no-console
console.log('Passed unknown unsubscribe token to unsubscribe', identifier);
}
}
if (identifier) {
tokens.delete(token);
const map = cache.get(identifier);
map?.delete(token);
const callbacks = cache.get(identifier);
if (!callbacks) {
return;
}

const index = callbacks.indexOf(token);
if (index === -1) {
assert(`Cannot unsubscribe a token that is not subscribed`, index !== -1);
return;
}

callbacks.splice(index, 1);
}
}

Expand All @@ -92,9 +106,8 @@ export default class NotificationManager {
declare _buffered: Map<StableDocumentIdentifier | StableRecordIdentifier, [string, string | undefined][]>;
declare _cache: Map<
StableDocumentIdentifier | StableRecordIdentifier | 'resource' | 'document',
Map<UnsubscribeToken, NotificationCallback | ResourceOperationCallback | DocumentOperationCallback>
Array<NotificationCallback | ResourceOperationCallback | DocumentOperationCallback>
>;
declare _tokens: Map<UnsubscribeToken, StableDocumentIdentifier | StableRecordIdentifier | 'resource' | 'document'>;
declare _hasFlush: boolean;
declare _onFlushCB?: () => void;

Expand All @@ -104,7 +117,6 @@ export default class NotificationManager {
this._buffered = new Map();
this._hasFlush = false;
this._cache = new Map();
this._tokens = new Map();
}

/**
Expand Down Expand Up @@ -148,17 +160,20 @@ export default class NotificationManager {
isStableIdentifier(identifier) ||
isDocumentIdentifier(identifier)
);
let map = this._cache.get(identifier);

if (!map) {
map = new Map();
this._cache.set(identifier, map);
let callbacks = this._cache.get(identifier);
assert(`expected to receive a valid callback`, typeof callback === 'function');
assert(`cannot subscribe with the same callback twice`, !callbacks || !callbacks.includes(callback));
// we use the callback as the cancellation token
//@ts-expect-error
callback.for = identifier;

if (!callbacks) {
callbacks = [];
this._cache.set(identifier, callbacks);
}

const unsubToken = DEBUG ? { _tokenRef: tokenId++ } : {};
map.set(unsubToken, callback);
this._tokens.set(unsubToken, identifier);
return unsubToken;
callbacks.push(callback);
return callback;
}

/**
Expand All @@ -170,7 +185,7 @@ export default class NotificationManager {
*/
unsubscribe(token: UnsubscribeToken) {
if (!this.isDestroyed) {
_unsubscribe(this._tokens, token, this._cache);
_unsubscribe(token, this._cache);
}
}

Expand Down Expand Up @@ -210,7 +225,7 @@ export default class NotificationManager {
return false;
}

const hasSubscribers = Boolean(this._cache.get(identifier)?.size);
const hasSubscribers = Boolean(this._cache.get(identifier)?.length);

if (isCacheOperationValue(value) || hasSubscribers) {
let buffer = this._buffered.get(identifier);
Expand Down Expand Up @@ -302,8 +317,7 @@ export default class NotificationManager {

// TODO for documents this will need to switch based on Identifier kind
if (isCacheOperationValue(value)) {
const callbackMap = this._cache.get(isDocumentIdentifier(identifier) ? 'document' : 'resource') as Map<
UnsubscribeToken,
const callbackMap = this._cache.get(isDocumentIdentifier(identifier) ? 'document' : 'resource') as Array<
ResourceOperationCallback | DocumentOperationCallback
>;

Expand All @@ -314,11 +328,11 @@ export default class NotificationManager {
}
}

const callbackMap = this._cache.get(identifier);
if (!callbackMap || !callbackMap.size) {
const callbacks = this._cache.get(identifier);
if (!callbacks || !callbacks.length) {
return false;
}
callbackMap.forEach((cb) => {
callbacks.forEach((cb) => {
// @ts-expect-error overload doesn't narrow within body
cb(identifier, value, key);
});
Expand All @@ -327,7 +341,6 @@ export default class NotificationManager {

destroy() {
this.isDestroyed = true;
this._tokens.clear();
this._cache.clear();
}
}
111 changes: 107 additions & 4 deletions packages/store/src/-private/store-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import { dependencySatisfies, importSync, macroCondition } from '@embroider/macr

import type RequestManager from '@ember-data/request';
import type { Future } from '@ember-data/request';
import { LOG_METRIC_COUNTS, LOG_PAYLOADS, LOG_REQUESTS } from '@warp-drive/build-config/debugging';
import {
__INTERNAL_LOG_NATIVE_MAP_SET_COUNTS,
LOG_METRIC_COUNTS,
LOG_PAYLOADS,
LOG_REQUESTS,
} from '@warp-drive/build-config/debugging';
import {
DEPRECATE_STORE_EXTENDS_EMBER_OBJECT,
ENABLE_LEGACY_SCHEMA_SERVICE,
Expand Down Expand Up @@ -70,16 +75,114 @@ globalThis.setWarpDriveLogging = setLogging;
globalThis.getWarpDriveRuntimeConfig = getRuntimeConfig;

if (LOG_METRIC_COUNTS) {
// @ts-expect-error
// @ts-expect-error adding to globalThis
// eslint-disable-next-line
globalThis.__WarpDriveMetricCountData = globalThis.__WarpDriveMetricCountData || {};

// @ts-expect-error
// @ts-expect-error adding to globalThis
globalThis.getWarpDriveMetricCounts = () => {
// @ts-expect-error
// eslint-disable-next-line
return globalThis.__WarpDriveMetricCountData;
return structuredClone(globalThis.__WarpDriveMetricCountData);
};

// @ts-expect-error adding to globalThis
globalThis.resetWarpDriveMetricCounts = () => {
// @ts-expect-error
globalThis.__WarpDriveMetricCountData = {};
};

if (__INTERNAL_LOG_NATIVE_MAP_SET_COUNTS) {
// @ts-expect-error adding to globalThis
globalThis.__primitiveInstanceId = 0;

function interceptAndLog(
klassName: 'Set' | 'Map' | 'WeakSet' | 'WeakMap',
methodName: 'add' | 'set' | 'delete' | 'has' | 'get' | 'constructor'
) {
const klass = globalThis[klassName];

if (methodName === 'constructor') {
const instantiationLabel = `new ${klassName}()`;
// @ts-expect-error
globalThis[klassName] = class extends klass {
// @ts-expect-error
constructor(...args) {
// eslint-disable-next-line
super(...args);
// @ts-expect-error

const instanceId = globalThis.__primitiveInstanceId++;
// @ts-expect-error
// eslint-disable-next-line
globalThis.__WarpDriveMetricCountData[instantiationLabel] =
// @ts-expect-error
// eslint-disable-next-line
(globalThis.__WarpDriveMetricCountData[instantiationLabel] || 0) + 1;
// @ts-expect-error
this.instanceName = `${klassName}:${instanceId} - ${new Error().stack?.split('\n')[2]}`;
}
};
} else {
// @ts-expect-error
// eslint-disable-next-line
const original = klass.prototype[methodName];
const logName = `${klassName}.${methodName}`;

// @ts-expect-error
klass.prototype[methodName] = function (...args) {
// @ts-expect-error
// eslint-disable-next-line
globalThis.__WarpDriveMetricCountData[logName] = (globalThis.__WarpDriveMetricCountData[logName] || 0) + 1;
// @ts-expect-error
const { instanceName } = this;
if (!instanceName) {
// @ts-expect-error
const instanceId = globalThis.__primitiveInstanceId++;
// @ts-expect-error
this.instanceName = `${klassName}.${methodName}:${instanceId} - ${new Error().stack?.split('\n')[2]}`;
}
const instanceLogName = `${logName} (${instanceName})`;
// @ts-expect-error
// eslint-disable-next-line
globalThis.__WarpDriveMetricCountData[instanceLogName] =
// @ts-expect-error
// eslint-disable-next-line
(globalThis.__WarpDriveMetricCountData[instanceLogName] || 0) + 1;
// eslint-disable-next-line
return original.apply(this, args);
};
}
}

interceptAndLog('Set', 'constructor');
interceptAndLog('Set', 'add');
interceptAndLog('Set', 'delete');
interceptAndLog('Set', 'has');
interceptAndLog('Set', 'set');
interceptAndLog('Set', 'get');

interceptAndLog('Map', 'constructor');
interceptAndLog('Map', 'set');
interceptAndLog('Map', 'delete');
interceptAndLog('Map', 'has');
interceptAndLog('Map', 'add');
interceptAndLog('Map', 'get');

interceptAndLog('WeakSet', 'constructor');
interceptAndLog('WeakSet', 'add');
interceptAndLog('WeakSet', 'delete');
interceptAndLog('WeakSet', 'has');
interceptAndLog('WeakSet', 'set');
interceptAndLog('WeakSet', 'get');

interceptAndLog('WeakMap', 'constructor');
interceptAndLog('WeakMap', 'set');
interceptAndLog('WeakMap', 'delete');
interceptAndLog('WeakMap', 'has');
interceptAndLog('WeakMap', 'add');
interceptAndLog('WeakMap', 'get');
}
}

export { storeFor };
Expand Down
4 changes: 4 additions & 0 deletions tests/performance/app/app.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// uncomment this to install Map/Set instrumentation
// prior to app boot
// import './services/store';

import Application from '@ember/application';

import compatModules from '@embroider/virtual/compat-modules';
Expand Down
1 change: 1 addition & 0 deletions tests/performance/ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module.exports = async function (defaults) {
// LOG_NOTIFICATIONS: true,
// LOG_INSTANCE_CACHE: true,
// LOG_METRIC_COUNTS: true,
// __INTERNAL_LOG_NATIVE_MAP_SET_COUNTS: true,
// DEBUG_RELATIONSHIP_NOTIFICATIONS: true,
},
});
Expand Down
Loading