From 35a3b5fec38c9844b7fc573602b443419a5aa47b Mon Sep 17 00:00:00 2001 From: Reto Kaiser Date: Mon, 18 May 2020 07:50:57 +0200 Subject: [PATCH] perf: Optimized version of EntityMetadata#compareIds() for the common case (#5419) * perf: Optimized version of EntityMetadata#compareIds() for the common case * Extract `compareIds()` into `OrmUtils` and use it instead of `.deepCompare()` where applicable * Use optimized path in compareIds() also when .id has type "number" * Add return type to signature of deepCompare() and compareIds() * made a proper check Co-authored-by: Umed Khudoiberdiev --- src/metadata/EntityMetadata.ts | 15 ++----------- .../SubjectChangedColumnsComputer.ts | 3 +-- .../ManyToManySubjectBuilder.ts | 5 ++--- .../OneToManySubjectBuilder.ts | 2 +- .../OneToOneInverseSideSubjectBuilder.ts | 2 +- .../RawSqlResultsToEntityTransformer.ts | 2 +- src/util/OrmUtils.ts | 22 ++++++++++++++++++- 7 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/metadata/EntityMetadata.ts b/src/metadata/EntityMetadata.ts index d4da9d2f618..20cb81ad807 100644 --- a/src/metadata/EntityMetadata.ts +++ b/src/metadata/EntityMetadata.ts @@ -618,7 +618,7 @@ export class EntityMetadata { const secondEntityIdMap = this.getEntityIdMap(secondEntity); if (!secondEntityIdMap) return false; - return EntityMetadata.compareIds(firstEntityIdMap, secondEntityIdMap); + return OrmUtils.compareIds(firstEntityIdMap, secondEntityIdMap); } /** @@ -739,21 +739,10 @@ export class EntityMetadata { */ static difference(firstIdMaps: ObjectLiteral[], secondIdMaps: ObjectLiteral[]): ObjectLiteral[] { return firstIdMaps.filter(firstIdMap => { - return !secondIdMaps.find(secondIdMap => OrmUtils.deepCompare(firstIdMap, secondIdMap)); + return !secondIdMaps.find(secondIdMap => OrmUtils.compareIds(firstIdMap, secondIdMap)); }); } - /** - * Compares ids of the two entities. - * Returns true if they match, false otherwise. - */ - static compareIds(firstId: ObjectLiteral|undefined, secondId: ObjectLiteral|undefined): boolean { - if (firstId === undefined || firstId === null || secondId === undefined || secondId === null) - return false; - - return OrmUtils.deepCompare(firstId, secondId); - } - /** * Creates value map from the given values and columns. * Examples of usages are primary columns map and join columns map. diff --git a/src/persistence/SubjectChangedColumnsComputer.ts b/src/persistence/SubjectChangedColumnsComputer.ts index c9d9fdf4d5a..0680aa6f2fe 100644 --- a/src/persistence/SubjectChangedColumnsComputer.ts +++ b/src/persistence/SubjectChangedColumnsComputer.ts @@ -1,7 +1,6 @@ import {Subject} from "./Subject"; import {DateUtils} from "../util/DateUtils"; import {ObjectLiteral} from "../common/ObjectLiteral"; -import {EntityMetadata} from "../metadata/EntityMetadata"; import {OrmUtils} from "../util/OrmUtils"; import {ApplyValueTransformers} from "../util/ApplyValueTransformers"; @@ -173,7 +172,7 @@ export class SubjectChangedColumnsComputer { const databaseRelatedEntityRelationIdMap = relation.getEntityValue(subject.databaseEntity); // if relation ids are equal then we don't need to update anything - const areRelatedIdsEqual = EntityMetadata.compareIds(relatedEntityRelationIdMap, databaseRelatedEntityRelationIdMap); + const areRelatedIdsEqual = OrmUtils.compareIds(relatedEntityRelationIdMap, databaseRelatedEntityRelationIdMap); if (areRelatedIdsEqual) { return; } else { diff --git a/src/persistence/subject-builder/ManyToManySubjectBuilder.ts b/src/persistence/subject-builder/ManyToManySubjectBuilder.ts index 850af6ef397..7d896cdc681 100644 --- a/src/persistence/subject-builder/ManyToManySubjectBuilder.ts +++ b/src/persistence/subject-builder/ManyToManySubjectBuilder.ts @@ -2,7 +2,6 @@ import {Subject} from "../Subject"; import {OrmUtils} from "../../util/OrmUtils"; import {ObjectLiteral} from "../../common/ObjectLiteral"; import {RelationMetadata} from "../../metadata/RelationMetadata"; -import {EntityMetadata} from "../../metadata/EntityMetadata"; /** * Builds operations needs to be executed for many-to-many relations of the given subjects. @@ -150,7 +149,7 @@ export class ManyToManySubjectBuilder { // try to find related entity in the database // by example: find post's category in the database post's categories const relatedEntityExistInDatabase = databaseRelatedEntityIds.find(databaseRelatedEntityRelationId => { - return EntityMetadata.compareIds(databaseRelatedEntityRelationId, relatedEntityRelationIdMap); + return OrmUtils.compareIds(databaseRelatedEntityRelationId, relatedEntityRelationIdMap); }); // if entity is found then don't do anything - it means binding in junction table already exist, we don't need to add anything @@ -207,7 +206,7 @@ export class ManyToManySubjectBuilder { // now from all entities in the persisted entity find only those which aren't found in the db const removedJunctionEntityIds = databaseRelatedEntityIds.filter(existRelationId => { return !changedInverseEntityRelationIds.find(changedRelationId => { - return EntityMetadata.compareIds(changedRelationId, existRelationId); + return OrmUtils.compareIds(changedRelationId, existRelationId); }); }); diff --git a/src/persistence/subject-builder/OneToManySubjectBuilder.ts b/src/persistence/subject-builder/OneToManySubjectBuilder.ts index 25250dfc5a7..2578bf1cdbd 100644 --- a/src/persistence/subject-builder/OneToManySubjectBuilder.ts +++ b/src/persistence/subject-builder/OneToManySubjectBuilder.ts @@ -117,7 +117,7 @@ export class OneToManySubjectBuilder { // check if this binding really exist in the database // by example: find our category if its already bind in the database const relationIdInDatabaseSubjectRelation = relatedEntityDatabaseRelationIds.find(relatedDatabaseEntityRelationId => { - return OrmUtils.deepCompare(relationIdMap, relatedDatabaseEntityRelationId); + return OrmUtils.compareIds(relationIdMap, relatedDatabaseEntityRelationId); }); // if relationIdMap DOES NOT exist in the subject's relation in the database it means its a new relation and we need to "bind" them diff --git a/src/persistence/subject-builder/OneToOneInverseSideSubjectBuilder.ts b/src/persistence/subject-builder/OneToOneInverseSideSubjectBuilder.ts index f421c5abc7f..6912768fdd5 100644 --- a/src/persistence/subject-builder/OneToOneInverseSideSubjectBuilder.ts +++ b/src/persistence/subject-builder/OneToOneInverseSideSubjectBuilder.ts @@ -139,7 +139,7 @@ export class OneToOneInverseSideSubjectBuilder { // check if this binding really exist in the database // by example: find our post if its already bind to category in the database and its not equal to what user tries to set - const areRelatedIdEqualWithDatabase = relatedEntityDatabaseRelationId && OrmUtils.deepCompare(relationIdMap, relatedEntityDatabaseRelationId); + const areRelatedIdEqualWithDatabase = relatedEntityDatabaseRelationId && OrmUtils.compareIds(relationIdMap, relatedEntityDatabaseRelationId); // if they aren't equal it means its a new relation and we need to "bind" them // by example: this will tell category to insert into its post relation our post we are working with diff --git a/src/query-builder/transformer/RawSqlResultsToEntityTransformer.ts b/src/query-builder/transformer/RawSqlResultsToEntityTransformer.ts index c10e46facd2..89118a4adbd 100644 --- a/src/query-builder/transformer/RawSqlResultsToEntityTransformer.ts +++ b/src/query-builder/transformer/RawSqlResultsToEntityTransformer.ts @@ -213,7 +213,7 @@ export class RawSqlResultsToEntityTransformer { const idMaps = rawRelationIdResult.results.map(result => { const entityPrimaryIds = this.extractEntityPrimaryIds(relation, result); - if (EntityMetadata.compareIds(entityPrimaryIds, valueMap) === false) + if (OrmUtils.compareIds(entityPrimaryIds, valueMap) === false) return; let columns: ColumnMetadata[]; diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts index 87aad7ea687..39e4fd9cbf4 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -102,7 +102,7 @@ export class OrmUtils { * * @see http://stackoverflow.com/a/1144249 */ - static deepCompare(...args: any[]) { + static deepCompare(...args: any[]): boolean { let i: any, l: any, leftChain: any, rightChain: any; if (arguments.length < 1) { @@ -123,6 +123,26 @@ export class OrmUtils { return true; } + /** + * Check if two entity-id-maps are the same + */ + static compareIds(firstId: ObjectLiteral|undefined, secondId: ObjectLiteral|undefined): boolean { + if (firstId === undefined || firstId === null || secondId === undefined || secondId === null) + return false; + + // Optimized version for the common case + if ( + ((typeof firstId.id === "string" && typeof secondId.id === "string") || + (typeof firstId.id === "number" && typeof secondId.id === "number")) && + Object.keys(firstId).length === 1 && + Object.keys(secondId).length === 1 + ) { + return firstId.id === secondId.id; + } + + return OrmUtils.deepCompare(firstId, secondId); + } + /** * Transforms given value into boolean value. */