perf(core): fix eager loading of multiple properties causing a cycles

Closes mikro-orm#4213

packages/core/src/entity/EntityLoader.ts

@@ -270,6 +270,7 @@ export class EntityLoader {
   }
 
   private async findChildren<T extends object>(entities: T[], prop: EntityProperty<T>, populate: PopulateOptions<T>, options: Required<EntityLoaderOptions<T>>): Promise<AnyEntity[]> {
+    entities = entities.filter(e => !helper(e).__loadedRelations.has(prop.name));
     const children = this.getChildReferences<T>(entities, prop, options.refresh);
     const meta = this.metadata.find(prop.type)!;
     let fk = Utils.getPrimaryKeyHash(meta.primaryKeys);
@@ -297,6 +298,7 @@ export class EntityLoader {
     const where = this.mergePrimaryCondition<T>(ids, fk as FilterKey<T>, options, meta, this.metadata, this.driver.getPlatform());
     const fields = this.buildFields(options.fields, prop);
     const { refresh, filters, convertCustomTypes, lockMode, strategy, populateWhere, connectionType } = options;
+    entities.forEach(e => !helper(e).__loadedRelations.add(prop.name));
 
     return this.em.find(prop.type, where, {
       refresh, filters, convertCustomTypes, lockMode, populateWhere,
@@ -576,7 +578,7 @@ export class EntityLoader {
         if (nested.length > 0) {
           ret.push(...nested);
         } else {
-          const selfReferencing = [meta.className, meta.root.className].includes(prop.type) && prop.eager;
+          const selfReferencing = [meta.className, meta.root.className, ...visited].includes(prop.type) && prop.eager;
           ret.push({
             field: prefixed,
             // enforce select-in strategy for self-referencing relations

packages/core/src/entity/WrappedEntity.ts

@@ -27,6 +27,7 @@ export class WrappedEntity<T extends object> {
   __em?: EntityManager;
   __serializationContext: { root?: SerializationContext<T>; populate?: PopulateOptions<T>[] } = {};
   __loadedProperties = new Set<string>();
+  __loadedRelations = new Set<string>();
   __data: Dictionary = {};
   __processing = false;
 

packages/core/src/typings.ts

@@ -147,6 +147,7 @@ export interface IWrappedEntityInternal<
   __touched: boolean;
   __originalEntityData?: EntityData<T>;
   __loadedProperties: Set<string>;
+  __loadedRelations: Set<string>;
   __identifier?: EntityIdentifier;
   __managed: boolean;
   __processing: boolean;

tests/perf/eager-loading-cycle.test.ts

@@ -0,0 +1,152 @@
+import { MikroORM } from '@mikro-orm/better-sqlite';
+import { Collection, Entity, ManyToOne, OneToMany, PrimaryKey, Property, Ref, ref, LoadStrategy } from '@mikro-orm/core';
+
+@Entity()
+class Author {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  name: string;
+
+  @Property({ unique: true })
+  email: string;
+
+  @OneToMany({
+    entity: () => Publisher,
+    mappedBy: p => p.bestSellingAuthor,
+    eager: true,
+  })
+  publishers = new Collection<Publisher>(this);
+
+  @OneToMany({
+    entity: () => Book,
+    mappedBy: b => b.author,
+  })
+  books = new Collection<Book>(this);
+
+  constructor(name: string, email: string) {
+    this.name = name;
+    this.email = email;
+  }
+
+}
+
+@Entity()
+class Book {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  title: string;
+
+  @ManyToOne({ entity: () => Author, ref: true, eager: true })
+  author: Ref<Author>;
+
+  @ManyToOne(() => Publisher, { ref: true, nullable: true })
+  publisher?: Ref<Publisher>;
+
+  constructor({ author, publisher, title }: {
+    author: Author;
+    publisher: Publisher;
+    title: string;
+  }) {
+    this.author = ref(author);
+    this.publisher = ref(publisher);
+    this.title = title;
+  }
+
+}
+
+@Entity()
+class Publisher {
+
+  @PrimaryKey()
+  id!: number;
+
+  @Property()
+  name: string;
+
+  @Property()
+  country!: string;
+
+  @ManyToOne({ entity: () => Author, ref: true })
+  bestSellingAuthor: Ref<Author>;
+
+  @OneToMany({
+    entity: () => Book,
+    mappedBy: a => a.publisher,
+    eager: true,
+  })
+  books = new Collection<Book>(this);
+
+  constructor(name: string, bestSellingAuthor: Author) {
+    this.name = name;
+    this.bestSellingAuthor = ref(bestSellingAuthor);
+  }
+
+}
+
+async function seed(orm: MikroORM) {
+  const em = orm.em.fork();
+  const author = em.create(Author, {
+    name: `God`,
+    email: `god@heaven.com`,
+  });
+
+  const publisher = em.create(Publisher, {
+    name: `pub`,
+    country: 'neverland',
+    bestSellingAuthor: author,
+  });
+
+  for (let i = 0; i < 50; i += 1) {
+    em.persist(
+      new Book({
+        author,
+        publisher,
+        title: `Bible pt.${i}`,
+      }),
+    );
+  }
+  await em.flush();
+  em.clear();
+}
+
+let orm: MikroORM;
+
+beforeAll(async () => {
+  orm = await MikroORM.init({
+    entities: [Author],
+    dbName: ':memory:',
+  });
+  await orm.schema.createSchema();
+  await seed(orm);
+});
+
+beforeEach(() => orm.em.clear());
+afterAll(() => orm.close(true));
+
+// should run around 15ms when run separately (M1 Pro 32g)
+test('perf: eager loading with cycles (select-in)', async () => {
+  console.time('perf: eager loading with cycles');
+  const res = await orm.em.find(Book, {}, { strategy: LoadStrategy.SELECT_IN });
+  console.timeEnd('perf: eager loading with cycles');
+
+  expect(res).toHaveLength(50);
+  expect(res[0].author.unwrap().publishers[0].books[0].title).toBe('Bible pt.0');
+  expect(res[0]).toBe(res[0].author.unwrap().publishers[0].books[0]);
+});
+
+// should run around 15ms when run separately (M1 Pro 32g)
+test('perf: eager loading with cycles (joined)', async () => {
+  console.time('perf: eager loading with cycles');
+  const res = await orm.em.find(Book, {}, { strategy: LoadStrategy.JOINED });
+  console.timeEnd('perf: eager loading with cycles');
+
+  expect(res).toHaveLength(50);
+  expect(res[0].author.unwrap().publishers[0].books[0].title).toBe('Bible pt.0');
+  expect(res[0]).toBe(res[0].author.unwrap().publishers[0].books[0]);
+});