perf(core): improve support for large collections (#3573)

packages/core/src/entity/ArrayCollection.ts

@@ -64,8 +64,10 @@ export class ArrayCollection<T extends object, O extends object> {
     }) as unknown as U[];
   }
 
-  add(...items: (T | Reference<T>)[]): void {
-    for (const item of items) {
+  add(entity: T | Reference<T> | (T | Reference<T>)[], ...entities: (T | Reference<T>)[]): void {
+    entities = Utils.asArray(entity).concat(entities);
+
+    for (const item of entities) {
       const entity = Reference.unwrapReference(item) as T;
 
       if (!this.contains(entity, false)) {
@@ -78,8 +80,8 @@ export class ArrayCollection<T extends object, O extends object> {
   }
 
   set(items: (T | Reference<T>)[]): void {
-    this.remove(...this.items);
-    this.add(...items);
+    this.removeAll();
+    this.add(items);
   }
 
   /**
@@ -92,7 +94,7 @@ export class ArrayCollection<T extends object, O extends object> {
 
     this.items.clear();
     this._count = 0;
-    this.add(...items);
+    this.add(items);
   }
 
   /**
@@ -101,8 +103,11 @@ export class ArrayCollection<T extends object, O extends object> {
    * is not the same as `em.remove()`. If we want to delete the entity by removing it from collection, we need to enable `orphanRemoval: true`,
    * which tells the ORM we don't want orphaned entities to exist, so we know those should be removed.
    */
-  remove(...items: (T | Reference<T>)[]): void {
-    for (const item of items) {
+  remove(entity: T | Reference<T> | (T | Reference<T>)[], ...entities: (T | Reference<T>)[]): void {
+    entities = Utils.asArray(entity).concat(entities);
+    let modified = false;
+
+    for (const item of entities) {
       if (!item) {
         continue;
       }
@@ -112,10 +117,14 @@ export class ArrayCollection<T extends object, O extends object> {
       if (this.items.delete(entity)) {
         this.incrementCount(-1);
         delete this[this.items.size]; // remove last item
-        Object.assign(this, [...this.items]); // reassign array access
         this.propagate(entity, 'remove');
+        modified ??= true;
       }
     }
+
+    if (modified) {
+      Object.assign(this, [...this.items]); // reassign array access
+    }
   }
 
   /**
@@ -125,7 +134,9 @@ export class ArrayCollection<T extends object, O extends object> {
    * which tells the ORM we don't want orphaned entities to exist, so we know those should be removed.
    */
   removeAll(): void {
-    this.remove(...this.items);
+    for (const item of this.items) {
+      this.remove(item);
+    }
   }
 
   /**
@@ -151,11 +162,17 @@ export class ArrayCollection<T extends object, O extends object> {
   }
 
   isInitialized(fully = false): boolean {
-    if (fully) {
-      return this.initialized && [...this.items].every((item: T) => helper(item).__initialized);
+    if (!this.initialized || !fully) {
+      return this.initialized;
+    }
+
+    for (const item of this.items) {
+      if (!helper(item).__initialized) {
+        return false;
+      }
     }
 
-    return this.initialized;
+    return true;
   }
 
   isDirty(): boolean {

packages/core/src/entity/Collection.ts

@@ -117,14 +117,15 @@ export class Collection<T extends object, O extends object = object> extends Arr
     return super.toJSON();
   }
 
-  add(...items: (T | Reference<T & AnyEntity>)[]): void {
-    const unwrapped = items.map(i => Reference.unwrapReference(i as AnyEntity)) as T[];
-    unwrapped.forEach(item => this.validateItemType(item));
+  add(entity: T | Reference<T> | T[], ...entities: (T | Reference<T>)[]): void {
+    entities = Utils.asArray(entity).concat(entities);
+    const unwrapped = entities.map(i => Reference.unwrapReference(i)) as T[];
+    unwrapped.forEach(entity => this.validateItemType(entity));
     this.modify('add', unwrapped);
     this.cancelOrphanRemoval(unwrapped);
   }
 
-  set(items: (T | Reference<T & AnyEntity>)[]): void {
+  set(items: (T | Reference<T>)[]): void {
     if (!this.initialized) {
       this.initialized = true;
       this.snapshot = undefined;
@@ -145,18 +146,19 @@ export class Collection<T extends object, O extends object = object> extends Arr
   /**
    * @inheritDoc
    */
-  remove(...items: (T | Reference<T & AnyEntity> | ((item: T) => boolean))[]): void {
-    if (items[0] instanceof Function) {
+  remove(entity: T | Reference<T> | (T | Reference<T>)[] | ((item: T) => boolean), ...entities: (T | Reference<T>)[]): void {
+    if (entity instanceof Function) {
       for (const item of this.items) {
-        if (items[0](item)) {
+        if (entity(item)) {
           this.remove(item);
         }
       }
 
       return;
     }
 
-    const unwrapped = items.map(i => Reference.unwrapReference(i as AnyEntity)) as T[];
+    entities = Utils.asArray(entity).concat(entities);
+    const unwrapped = entities.map(i => Reference.unwrapReference(i)) as T[];
     this.modify('remove', unwrapped);
     const em = this.getEntityManager(unwrapped, false);
 
@@ -167,7 +169,18 @@ export class Collection<T extends object, O extends object = object> extends Arr
     }
   }
 
-  contains(item: (T | Reference<T & AnyEntity>), check = true): boolean {
+  /**
+   * @inheritDoc
+   */
+  removeAll(): void {
+    this.checkInitialized();
+
+    for (const item of this.items) {
+      this.remove(item);
+    }
+  }
+
+  contains(item: T | Reference<T>, check = true): boolean {
     if (check) {
       this.checkInitialized();
     }
@@ -274,9 +287,9 @@ export class Collection<T extends object, O extends object = object> extends Arr
     let em = this._em ?? helper(this.owner).__em;
 
     if (!em) {
-      for (const i of items as AnyEntity[]) {
-        if (i?.__helper!.__em) {
-          em = i?.__helper!.__em;
+      for (const i of items) {
+        if (helper(i)?.__em) {
+          em = helper(i).__em;
           break;
         }
       }
@@ -319,7 +332,7 @@ export class Collection<T extends object, O extends object = object> extends Arr
       // we know there is at least one item as it was checked in load method
       const pk = this.property.targetMeta!.primaryKeys[0];
       cond[pk] = { $in: [] };
-      this.items.forEach(item => cond[pk].$in.push((item as AnyEntity).__helper!.getPrimaryKey()));
+      this.items.forEach(item => cond[pk].$in.push(helper(item).getPrimaryKey()));
     } else {
       cond[this.property.mappedBy] = helper(this.owner).getPrimaryKey();
     }
@@ -348,7 +361,7 @@ export class Collection<T extends object, O extends object = object> extends Arr
     }
 
     this.validateModification(items);
-    super[method](...items);
+    super[method](items);
     this.setDirty();
   }
 
@@ -404,7 +417,7 @@ export class Collection<T extends object, O extends object = object> extends Arr
     };
 
     // throw if we are modifying inverse side of M:N collection when owning side is initialized (would be ignored when persisting)
-    if (items.find(item => check(item as AnyEntity))) {
+    if (items.find(item => check(item))) {
       throw ValidationError.cannotModifyInverseCollection(this.owner, this.property);
     }
   }

packages/core/src/entity/EntityLoader.ts

@@ -10,6 +10,7 @@ import { Reference } from './Reference';
 import type { EntityField, FindOptions } from '../drivers/IDatabaseDriver';
 import type { MetadataStorage } from '../metadata/MetadataStorage';
 import type { Platform } from '../platforms/Platform';
+import { helper } from './wrap';
 
 export type EntityLoaderOptions<T, P extends string = never> = {
   where?: FilterQuery<T>;
@@ -220,10 +221,10 @@ export class EntityLoader {
   }
 
   private initializeOneToMany<T>(filtered: T[], children: AnyEntity[], prop: EntityProperty, field: keyof T): void {
-    for (const entity of (filtered as (T & AnyEntity)[])) {
+    for (const entity of filtered) {
       const items = children.filter(child => {
         if (prop.targetMeta!.properties[prop.mappedBy].mapToPk) {
-          return child[prop.mappedBy] as unknown === entity.__helper!.getPrimaryKey();
+          return child[prop.mappedBy] as unknown === helper(entity).getPrimaryKey();
         }
 
         return Reference.unwrapReference(child[prop.mappedBy]) as unknown === entity;
@@ -234,8 +235,8 @@ export class EntityLoader {
   }
 
   private initializeManyToMany<T>(filtered: T[], children: AnyEntity[], prop: EntityProperty, field: keyof T): void {
-    for (const entity of (filtered as (T & AnyEntity)[])) {
-      const items = children.filter(child => (child[prop.mappedBy] as unknown as Collection<AnyEntity>).contains(entity));
+    for (const entity of filtered) {
+      const items = children.filter(child => (child[prop.mappedBy] as unknown as Collection<AnyEntity>).contains(entity as AnyEntity));
       (entity[field] as unknown as Collection<AnyEntity>).hydrate(items, true);
     }
   }

tests/EntityManager.mysql.test.ts

@@ -1129,7 +1129,6 @@ describe('EntityManagerMySql', () => {
     expect(tags[0].books.isDirty()).toBe(false);
     expect(() => tags[0].books.getItems()).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
     expect(() => tags[0].books.remove(book1, book2)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
-    expect(() => tags[0].books.removeAll()).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
     expect(() => tags[0].books.contains(book1)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
 
     // test M:N lazy load

tests/EntityManager.postgre.test.ts

@@ -1882,7 +1882,7 @@ describe('EntityManagerPostgre', () => {
     expect(author.books.getItems().every(b => b.uuid)).toBe(true);
   });
 
-  // this should run in ~150ms (when running single test locally)
+  // this should run in ~70ms (when running single test locally)
   test('perf: one to many via em.create()', async () => {
     const books = [] as Book2[];
     for (let i = 1; i <= 10000; i++) {

tests/EntityManager.sqlite2.test.ts

@@ -1275,7 +1275,7 @@ describe.each(['sqlite', 'better-sqlite'] as const)('EntityManager (%s)', driver
     // n:m relations
     let taggedBook = orm.em.create(Book4, { title: 'FullyTagged' });
     await orm.em.persistAndFlush(taggedBook);
-    const tags = [orm.em.create(BookTag4, { name: 'science-fiction' }), orm.em.create(BookTag4, { name: 'adventure' }), orm.em.create(BookTag4, { name: 'horror' })];
+    const tags = [orm.em.create(BookTag4, { name: 'science-fiction' }), orm.em.create(BookTag4, { name: 'adventure' }), orm.em.create(BookTag4, { name: 'horror' })] as const;
     taggedBook.tags.add(...tags);
     await expect(taggedBook.tags.loadCount()).resolves.toEqual(0);
     await orm.em.flush();

tests/features/paginate-flag.postgres.test.ts

@@ -35,7 +35,7 @@ export class User {
   constructor(id: string, name: string, groups: Group[]) {
     this.id = id;
     this.name = name;
-    this.groups.add(...groups);
+    this.groups.add(groups);
   }
 
 }

tests/issues/GH910.test.ts

@@ -43,7 +43,7 @@ export class Cart {
 
   constructor(id: string, items: CartItem[]) {
     this.id = id;
-    this.items.add(...items);
+    this.items.add(items);
   }
 
   addItem(item: CartItem): void {