From 02d6e2d76f42047c88ec4396070d0ccf15b67564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Mon, 15 Jun 2026 14:22:44 +0200 Subject: [PATCH] =?UTF-8?q?perf(server):=20avoid=20O(n=C2=B2)=20when=20bui?= =?UTF-8?q?lding=20flat=20entity=20maps=20(#21585)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem After 2.13, server CPU stepped up and stayed up. Sentry profiling of `POST /metadata` pins it on `addFlatEntityToFlatEntityMapsThroughMutationOrThrow` — ~26% self-time plus a long tail, turning metadata-write requests into multi-second (~18s observed) operations. The hot stack is: ``` WorkspaceMigrationValidateBuildAndRunService.computeAllRelatedFlatEntityMaps └ getSubFlatEntityMapsByApplicationIdsOrThrow └ addFlatEntityToFlatEntityMapsThroughMutationOrThrow ``` Every metadata migration rebuilds the twenty-standard application's flat sub-maps — thousands of entities, across every involved metadata type — through this util. ## Root cause `addFlatEntityToFlatEntityMapsThroughMutationOrThrow` maintains `universalIdentifiersByApplicationId` and deduped each insert with `Array.includes`: ```ts if (!existingUniversalIdentifiers.includes(flatEntity.universalIdentifier)) { existingUniversalIdentifiers.push(flatEntity.universalIdentifier); } ``` That scan is O(n) per insert, so building a map for an application with `n` entities is **O(n²)**. The twenty-standard application groups thousands of standard entities under one `applicationId`, so its sub-map rebuild dominates. The dedup is also redundant: the function throws `ENTITY_ALREADY_EXISTS` at the top if the `universalIdentifier` is already in `byUniversalIdentifier`, and every id pushed to the per-application list is also written there. So reaching the push guarantees the id is new — the `.includes()` is always `false`. ## Fix Drop the scan and push directly → map building is **O(n)**. Behavior is unchanged (the early throw already enforces uniqueness). ## Test Adds a unit spec covering indexing, the no-`applicationId` case, the duplicate throw, and a 20k-entity build that completes instantly (guards against re-introducing the quadratic). ## Follow-up (separate PR) This is the bleed-stopper. The deeper issue is that `computeAllInvolvedApplicationIds` pulls the **entire** twenty-standard application into the dependency set of every migration and rebuilds those sub-maps per request instead of caching them. Worth scoping the dependency set to referenced entities (or caching the standard-app sub-maps), which I'll raise separately. Review in cubic --- ...aps-through-mutation-or-throw.util.spec.ts | 112 ++++++++++++++++++ ...ity-maps-through-mutation-or-throw.util.ts | 6 +- 2 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/__tests__/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.spec.ts diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/__tests__/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.spec.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/__tests__/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.spec.ts new file mode 100644 index 0000000000..e0c728770e --- /dev/null +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/__tests__/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.spec.ts @@ -0,0 +1,112 @@ +import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant'; +import { FlatEntityMapsExceptionCode } from 'src/engine/metadata-modules/flat-entity/exceptions/flat-entity-maps.exception'; +import { type SyncableFlatEntity } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-from.type'; +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +import { addFlatEntityToFlatEntityMapsThroughMutationOrThrow } from 'src/engine/workspace-manager/workspace-migration/utils/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util'; + +const createMaps = (): FlatEntityMaps => + createEmptyFlatEntityMaps(); + +const makeFlatEntity = ( + id: string, + universalIdentifier: string, + applicationId?: string, +): SyncableFlatEntity => + ({ + id, + universalIdentifier, + applicationId, + }) as unknown as SyncableFlatEntity; + +describe('addFlatEntityToFlatEntityMapsThroughMutationOrThrow', () => { + it('should index a new entity by universalIdentifier and id', () => { + const flatEntityMapsToMutate = createMaps(); + const flatEntity = makeFlatEntity('id-1', 'uid-1', 'app-1'); + + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity, + flatEntityMapsToMutate, + }); + + expect(flatEntityMapsToMutate.byUniversalIdentifier['uid-1']).toBe( + flatEntity, + ); + expect(flatEntityMapsToMutate.universalIdentifierById['id-1']).toBe( + 'uid-1', + ); + expect( + flatEntityMapsToMutate.universalIdentifiersByApplicationId['app-1'], + ).toEqual(['uid-1']); + }); + + it('should append entities that share an applicationId without duplicating', () => { + const flatEntityMapsToMutate = createMaps(); + + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity: makeFlatEntity('id-1', 'uid-1', 'app-1'), + flatEntityMapsToMutate, + }); + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity: makeFlatEntity('id-2', 'uid-2', 'app-1'), + flatEntityMapsToMutate, + }); + + const universalIdentifiers = + flatEntityMapsToMutate.universalIdentifiersByApplicationId['app-1']; + + expect(universalIdentifiers).toEqual(['uid-1', 'uid-2']); + // Distinct entities, so each universalIdentifier appears exactly once + expect(new Set(universalIdentifiers).size).toBe( + universalIdentifiers?.length, + ); + }); + + it('should not index entities that have no applicationId', () => { + const flatEntityMapsToMutate = createMaps(); + + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity: makeFlatEntity('id-1', 'uid-1'), + flatEntityMapsToMutate, + }); + + expect(flatEntityMapsToMutate.universalIdentifiersByApplicationId).toEqual( + {}, + ); + }); + + it('should throw when the universalIdentifier already exists', () => { + const flatEntityMapsToMutate = createMaps(); + + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity: makeFlatEntity('id-1', 'uid-1', 'app-1'), + flatEntityMapsToMutate, + }); + + expect(() => + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity: makeFlatEntity('id-1-bis', 'uid-1', 'app-1'), + flatEntityMapsToMutate, + }), + ).toThrow( + expect.objectContaining({ + code: FlatEntityMapsExceptionCode.ENTITY_ALREADY_EXISTS, + }), + ); + }); + + it('should retain every identifier when many entities share an applicationId', () => { + const flatEntityMapsToMutate = createMaps(); + const entityCount = 20_000; + + for (let index = 0; index < entityCount; index++) { + addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ + flatEntity: makeFlatEntity(`id-${index}`, `uid-${index}`, 'app-1'), + flatEntityMapsToMutate, + }); + } + + expect( + flatEntityMapsToMutate.universalIdentifiersByApplicationId['app-1'], + ).toHaveLength(entityCount); + }); +}); diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.ts index 3657a1c5a4..163bbf89ee 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration/utils/add-flat-entity-to-flat-entity-maps-through-mutation-or-throw.util.ts @@ -46,11 +46,7 @@ export const addFlatEntityToFlatEntityMapsThroughMutationOrThrow = < ]; if (isDefined(existingUniversalIdentifiers)) { - if ( - !existingUniversalIdentifiers.includes(flatEntity.universalIdentifier) - ) { - existingUniversalIdentifiers.push(flatEntity.universalIdentifier); - } + existingUniversalIdentifiers.push(flatEntity.universalIdentifier); } else { flatEntityMapsToMutate.universalIdentifiersByApplicationId[ flatEntity.applicationId