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

Flush performance issue on a collection #4807

Closed
clero opened this issue Oct 10, 2023 · 2 comments
Closed

Flush performance issue on a collection #4807

clero opened this issue Oct 10, 2023 · 2 comments

Comments

@clero
Copy link

clero commented Oct 10, 2023

Description
My team and I recently had big performance issues on our app. We managed to fix them after refactoring several collections management.
We usually write entity constructors to always have a consistent model, thus a container (in the following example the sea) is often created with a bunch a elements (fish) passed directly in the constructor. We then persist and flush the container alongside all elements in one call.
It leads to bad performance. Creating and persisting/flushing an empty container which will then be filled with elements is 10 times faster.

To Reproduce
Steps to reproduce the behavior:

  1. Launch this snippet with jest and compare elapsed time. jest flush.spec.ts
/* eslint-disable max-classes-per-file, @typescript-eslint/no-use-before-define */
import { Collection, Entity, ManyToOne, MikroORM, OneToMany, PrimaryKey } from '@mikro-orm/core';
import type { SqliteDriver } from '@mikro-orm/sqlite';

@Entity()
class Sea {
  @PrimaryKey()
  id!: number;

  @OneToMany(() => Fish, ({ sea }) => sea)
  fishes = new Collection<Fish>(this);
}

@Entity()
class Fish {
  @PrimaryKey()
  id!: number;

  @ManyToOne(() => Sea)
  sea!: Sea;
}

describe('MikroORM Collection flush issue', () => {
  let orm: MikroORM<SqliteDriver>;
  const numberOfFishInTheSea = 10000;

  beforeEach(async () => {
    orm = await MikroORM.init({
      entities: [Sea, Fish],
      dbName: ':memory:',
      type: 'sqlite',
    });
    await orm.getSchemaGenerator().createSchema();
  });

  afterEach(() => orm.close(true));

  test('when persisting the whole model, it is slow', async () => {
    const entityManager = orm.em.fork();

    // so hard to write
    const mediterranean = new Sea();
    const groupers = Array.from({ length: numberOfFishInTheSea }).map(() => new Fish());

    mediterranean.fishes.add(groupers);

    // everything is ready let's persist
    await entityManager.persistAndFlush(mediterranean);
  });

  test('when flushing the container before the contained data, it is fast', async () => {
    const entityManager = orm.em.fork();

    // still so hard to write, but still nothing to do with our problem
    const mediterranean = new Sea();

    // Let's persist our sea before adding fish. It speed up the final flush...
    await entityManager.persistAndFlush(mediterranean);

    const groupers = Array.from({ length: numberOfFishInTheSea }).map(() => new Fish());

    mediterranean.fishes.add(groupers);

    await entityManager.flush();
  });
});

Expected behavior
I would expect similar performance in both case.

Additional context
Following old issue guided us towards the workaround: #2379
Is this a regression?

Versions

Dependency Version
node 18.17.1
typescript 4.3.5
mikro-orm 5.8.0
mikro-orm/postgresql 5.8.0
mikro-orm/sqlite 5.8.0
jest 28.1.2
@B4nan
Copy link
Member

B4nan commented Oct 11, 2023

Is this a regression?

Nope, that issue was closed with a test case which is still performing the same, it's probably something else (although very similar).

@B4nan B4nan closed this as completed in a8a1021 Oct 11, 2023
@clero
Copy link
Author

clero commented Oct 12, 2023

that was fast! thx @B4nan !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants