enhance: Improve performance by using Map (#3390)

* enhance: Improve performance by using Map

* enhance: Use maps in networkmanager

* enhance: Map in RestHelpers

* enhance: queryKeys to Map

* enhance: Map in addEntities

* internal: Changeset

* docs: Update benchmarks for new performance results

* enhance: Interceptors that have args specified will still work, and warn about args.

* enhance: Remove NM changes for now as they are breaking

Author: ntucker

.changeset/evil-beans-attack.md

@@ -0,0 +1,7 @@
+---
+'@data-client/normalizr': patch
+'@data-client/rest': patch
+'@data-client/test': patch
+---
+
+Improve performance by using Map() instead of Object for unbounded keys

.changeset/loud-rice-fry.md

@@ -0,0 +1,22 @@
+---
+'@data-client/test': patch
+---
+
+Interceptors that have args specified will still work, and warn about args.
+
+Example:
+
+```typescript
+{
+  endpoint: TodoResource.getList.push,
+  args: [{ userId: '5' }, {}],
+  response({ userId }, body) {
+    return { id: Math.random(), userId, ...ensurePojo(body) };
+  },
+}
+```
+
+This is clearly an interceptor, but args were accidentally specified. Before
+this would make it not register, and TypeScript couldn't detect the issue.
+
+Now this is treated as an interceptor (args ignored); and there is a console warning

docs/core/concepts/normalization.md

@@ -391,16 +391,16 @@ and up to 20x performance even after [mutation operations](../getting-started/mu
 xychart-beta
     title "Denormalize Single Entity"
     x-axis [normalizr, "Data Client", "Data Client (cached)"]
-    y-axis "Operations per second" 0 --> 4875500
-    bar [218500, 1079500, 4875500]
+    y-axis "Operations per second" 0 --> 5618500
+    bar [212500, 1288500, 5618500]
 ```
 
 ```mermaid
 xychart-beta
     title "Denormalize Large List"
     x-axis [normalizr, "Data Client", "Data Client (cached)"]
     y-axis "Operations per second" 0 --> 12962
-    bar [1165, 1807, 13168]
+    bar [1151, 1807, 13182]
 ```
 
 </Grid>

examples/benchmark/README.md

@@ -23,10 +23,10 @@ Performance compared to normalizr package (higher is better):
 
 |                     | no cache | with cache |
 | ------------------- | -------- | ---------- |
-| normalize (long)    | 121%     | 121%       |
-| denormalize (long)  | 158%     | 1,262%     |
-| denormalize (short) | 676%     | 2,367%     |
+| normalize (long)    | 120%     | 120%       |
+| denormalize (long)  | 158%     | 1,250%     |
+| denormalize (short) | 676%     | 2,650%     |
 
-[Comparison done on a Ryzen 7950x; Ubuntu; Node 20.10.0]
+[Comparison done on a Ryzen 7950x; Ubuntu; Node 22.14.0]
 
 Not only is denormalize faster, but it is more feature-rich as well.

packages/normalizr/README.md

@@ -313,9 +313,9 @@ Available from [@data-client/endpoint](https://www.npmjs.com/package/@data-clien
 
 |                     | no cache | with cache |
 | ------------------- | -------- | ---------- |
-| normalize (long)    | 121%     | 121%       |
-| denormalize (long)  | 158%     | 1,262%     |
-| denormalize (short) | 676%     | 2,367%     |
+| normalize (long)    | 120%     | 120%       |
+| denormalize (long)  | 158%     | 1,250%     |
+| denormalize (short) | 676%     | 2,650%     |
 
 [View benchmark](https://github.com/reactive/data-client/blob/master/examples/benchmark)
 

packages/normalizr/src/denormalize/cache.ts

@@ -6,7 +6,7 @@ export default interface Cache {
     pk: string,
     schema: EntityInterface,
     entity: any,
-    computeValue: (localCacheKey: Record<string, any>) => void,
+    computeValue: (localCacheKey: Map<string, any>) => void,
   ): object | undefined | symbol;
   getResults(
     input: any,

packages/normalizr/src/denormalize/localCache.ts

@@ -3,24 +3,24 @@ import type { EntityInterface } from '../interface.js';
 import type { EntityPath } from '../types.js';
 
 export default class LocalCache implements Cache {
-  private localCache: Record<string, Record<string, any>> = {};
+  private localCache = new Map<string, Map<string, any>>();
 
   getEntity(
     pk: string,
     schema: EntityInterface,
     entity: any,
-    computeValue: (localCacheKey: Record<string, any>) => void,
+    computeValue: (localCacheKey: Map<string, any>) => void,
   ): object | undefined | symbol {
     const key = schema.key;
-    if (!(key in this.localCache)) {
-      this.localCache[key] = Object.create(null);
+    if (!this.localCache.has(key)) {
+      this.localCache.set(key, new Map<string, any>());
     }
-    const localCacheKey = this.localCache[key];
+    const localCacheKey = this.localCache.get(key) as Map<string, any>;
 
-    if (!localCacheKey[pk]) {
+    if (!localCacheKey.get(pk)) {
       computeValue(localCacheKey);
     }
-    return localCacheKey[pk];
+    return localCacheKey.get(pk);
   }
 
   getResults(

packages/normalizr/src/denormalize/unvisit.ts

@@ -35,7 +35,7 @@ function unvisitEntity(
     // we're actually using this call to ensure we update the cache if a nested schema changes from `undefined`
     // this is because cache.getEntity adds this key,pk as a dependency of anything it is nested under
     return cache.getEntity(entityOrId, schema, UNDEF, localCacheKey => {
-      localCacheKey[entityOrId] = undefined;
+      localCacheKey.set(entityOrId, undefined);
     });
   }
 
@@ -71,33 +71,34 @@ function unvisitEntity(
 }
 
 function noCacheGetEntity(
-  computeValue: (localCacheKey: Record<string, any>) => void,
+  computeValue: (localCacheKey: Map<string, any>) => void,
 ): object | undefined | symbol {
-  const localCacheKey = {};
+  const localCacheKey = new Map<string, any>();
   computeValue(localCacheKey);
 
-  return localCacheKey[''];
+  return localCacheKey.get('');
 }
 
 function unvisitEntityObject(
   entity: object,
   schema: EntityInterface<any>,
   unvisit: (schema: any, input: any) => any,
   pk: string,
-  localCacheKey: Record<string, any>,
+  localCacheKey: Map<string, any>,
   args: readonly any[],
 ): void {
-  const entityCopy = (localCacheKey[pk] =
+  const entityCopy =
     isImmutable(entity) ?
       schema.createIfValid(entity.toObject())
-    : schema.createIfValid(entity));
+    : schema.createIfValid(entity);
+  localCacheKey.set(pk, entityCopy);
 
   if (entityCopy === undefined) {
     // undefined indicates we should suspense (perhaps failed validation)
-    localCacheKey[pk] = INVALID;
+    localCacheKey.set(pk, INVALID);
   } else {
     if (typeof schema.denormalize === 'function') {
-      localCacheKey[pk] = schema.denormalize(entityCopy, args, unvisit);
+      localCacheKey.set(pk, schema.denormalize(entityCopy, args, unvisit));
     }
   }
 }

packages/normalizr/src/memo/MemoCache.ts

@@ -17,11 +17,11 @@ import type {
 /** Singleton to store the memoization cache for denormalization methods */
 export default class MemoCache {
   /** Cache for every entity based on its dependencies and its own input */
-  protected entities: EntityCache = {};
+  protected entities: EntityCache = new Map();
   /** Caches the final denormalized form based on input, entities */
   protected endpoints: EndpointsCache = new WeakDependencyMap<EntityPath>();
   /** Caches the queryKey based on schema, args, and any used entities or indexes */
-  protected queryKeys: Record<string, WeakDependencyMap<QueryPath>> = {};
+  protected queryKeys: Map<string, WeakDependencyMap<QueryPath>> = new Map();
 
   /** Compute denormalized form maintaining referential equality for same inputs */
   denormalize<S extends Schema>(
@@ -107,10 +107,14 @@ export default class MemoCache {
       return schema as any;
 
     // cache lookup: argsKey -> schema -> ...touched indexes or entities
-    if (!this.queryKeys[argsKey]) {
-      this.queryKeys[argsKey] = new WeakDependencyMap<QueryPath>();
+    if (!this.queryKeys.get(argsKey)) {
+      this.queryKeys.set(argsKey, new WeakDependencyMap<QueryPath>());
     }
-    const queryCache = this.queryKeys[argsKey];
+    const queryCache = this.queryKeys.get(argsKey) as WeakDependencyMap<
+      QueryPath,
+      object,
+      any
+    >;
     const getEntity = createGetEntity(entities);
     const getIndex = createGetIndex(indexes);
     // eslint-disable-next-line prefer-const

packages/normalizr/src/memo/globalCache.ts

@@ -7,9 +7,9 @@ import type { EntityPath } from '../types.js';
 
 export default class GlobalCache implements Cache {
   private dependencies: Dep<EntityPath>[] = [];
-  private cycleCache: Record<string, Record<string, number>> = {};
+  private cycleCache: Map<string, Map<string, number>> = new Map();
   private cycleIndex = -1;
-  private localCache: Record<string, Record<string, any>> = {};
+  private localCache: Map<string, Map<string, any>> = new Map();
 
   declare private getCache: (
     pk: string,
@@ -33,12 +33,12 @@ export default class GlobalCache implements Cache {
     pk: string,
     schema: EntityInterface,
     entity: any,
-    computeValue: (localCacheKey: Record<string, any>) => void,
+    computeValue: (localCacheKey: Map<string, any>) => void,
   ): object | undefined | symbol {
     const key = schema.key;
     const { localCacheKey, cycleCacheKey } = this.getCacheKey(key);
 
-    if (!localCacheKey[pk]) {
+    if (!localCacheKey.get(pk)) {
       const globalCache: WeakDependencyMap<
         EntityPath,
         object,
@@ -48,7 +48,7 @@ export default class GlobalCache implements Cache {
       // TODO: what if this just returned the deps - then we don't need to store them
 
       if (cachePath) {
-        localCacheKey[pk] = cacheValue.value;
+        localCacheKey.set(pk, cacheValue.value);
         // TODO: can we store the cache values instead of tracking *all* their sources?
         // this is only used for setting endpoints cache correctly. if we got this far we will def need to set as we would have already tried getting it
         this.dependencies.push(...cacheValue.dependencies);
@@ -57,22 +57,22 @@ export default class GlobalCache implements Cache {
       // if we don't find in denormalize cache then do full denormalize
       else {
         const trackingIndex = this.dependencies.length;
-        cycleCacheKey[pk] = trackingIndex;
+        cycleCacheKey.set(pk, trackingIndex);
         this.dependencies.push({ entity, path: { key, pk } });
 
         /** NON-GLOBAL_CACHE CODE */
         computeValue(localCacheKey);
         /** /END NON-GLOBAL_CACHE CODE */
 
-        delete cycleCacheKey[pk];
+        cycleCacheKey.delete(pk);
         // if in cycle, use the start of the cycle to track all deps
         // otherwise, we use our own trackingIndex
         const localKey = this.dependencies.slice(
           this.cycleIndex === -1 ? trackingIndex : this.cycleIndex,
         );
         const cacheValue: EntityCacheValue = {
           dependencies: localKey,
-          value: localCacheKey[pk],
+          value: localCacheKey.get(pk),
         };
         globalCache.set(localKey, cacheValue);
 
@@ -83,25 +83,25 @@ export default class GlobalCache implements Cache {
       }
     } else {
       // cycle detected
-      if (pk in cycleCacheKey) {
-        this.cycleIndex = cycleCacheKey[pk];
+      if (cycleCacheKey.has(pk)) {
+        this.cycleIndex = cycleCacheKey.get(pk)!;
       } else {
         // with no cycle, globalCacheEntry will have already been set
         this.dependencies.push({ entity, path: { key, pk } });
       }
     }
-    return localCacheKey[pk];
+    return localCacheKey.get(pk);
   }
 
   private getCacheKey(key: string) {
-    if (!(key in this.localCache)) {
-      this.localCache[key] = Object.create(null);
+    if (!this.localCache.has(key)) {
+      this.localCache.set(key, new Map());
     }
-    if (!(key in this.cycleCache)) {
-      this.cycleCache[key] = Object.create(null);
+    if (!this.cycleCache.has(key)) {
+      this.cycleCache.set(key, new Map());
     }
-    const localCacheKey = this.localCache[key];
-    const cycleCacheKey = this.cycleCache[key];
+    const localCacheKey = this.localCache.get(key)!;
+    const cycleCacheKey = this.cycleCache.get(key)!;
     return { localCacheKey, cycleCacheKey };
   }
 
@@ -150,22 +150,31 @@ const getEntityCaches = (entityCache: EntityCache) => {
     // TODO: this should be based on a public interface
     const entityInstance: EntityInterface = (schema.cacheWith as any) ?? schema;
 
-    if (!(key in entityCache)) {
-      entityCache[key] = Object.create(null);
+    if (!entityCache.has(key)) {
+      entityCache.set(key, new Map());
     }
-    const entityCacheKey = entityCache[key];
-    if (!entityCacheKey[pk])
-      entityCacheKey[pk] = new WeakMap<
-        EntityInterface,
-        WeakDependencyMap<EntityPath, object, any>
-      >();
-
-    let wem: WeakDependencyMap<EntityPath, object, any> = entityCacheKey[
-      pk
-    ].get(entityInstance) as any;
+    const entityCacheKey = entityCache.get(key)!;
+    if (!entityCacheKey.get(pk))
+      entityCacheKey.set(
+        pk,
+        new WeakMap<
+          EntityInterface,
+          WeakDependencyMap<EntityPath, object, any>
+        >(),
+      );
+
+    const entityCachePk = entityCacheKey.get(pk) as WeakMap<
+      EntityInterface<any>,
+      WeakDependencyMap<EntityPath, object, any>
+    >;
+    let wem = entityCachePk.get(entityInstance) as WeakDependencyMap<
+      EntityPath,
+      object,
+      any
+    >;
     if (!wem) {
       wem = new WeakDependencyMap<EntityPath, object, any>();
-      entityCacheKey[pk].set(entityInstance, wem);
+      entityCachePk.set(entityInstance, wem);
     }
 
     return wem;