From da8716c81c3b3506dbb6f0da3526dc87c2dd2b12 Mon Sep 17 00:00:00 2001 From: Reto Kaiser Date: Sun, 26 Jan 2020 19:44:42 +0100 Subject: [PATCH 1/5] perf: Optimized version of EntityMetadata#compareIds() for the common case --- src/metadata/EntityMetadata.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/metadata/EntityMetadata.ts b/src/metadata/EntityMetadata.ts index 7b5e0ae8bf..d91d32ca06 100644 --- a/src/metadata/EntityMetadata.ts +++ b/src/metadata/EntityMetadata.ts @@ -746,6 +746,16 @@ export class EntityMetadata { 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" && + Object.keys(firstId).length === 1 && + Object.keys(secondId).length === 1 + ) { + return firstId.id === secondId.id; + } + return OrmUtils.deepCompare(firstId, secondId); } From 4bd2e347f5a0da2ba4a7db0efdf36a6cfc9910bb Mon Sep 17 00:00:00 2001 From: Reto Kaiser Date: Mon, 27 Jan 2020 19:33:41 +0100 Subject: [PATCH 2/5] Extract `compareIds()` into `OrmUtils` and use it instead of `.deepCompare()` where applicable --- src/metadata/EntityMetadata.ts | 25 ++----------------- .../SubjectChangedColumnsComputer.ts | 3 +-- .../ManyToManySubjectBuilder.ts | 5 ++-- .../OneToManySubjectBuilder.ts | 2 +- .../OneToOneInverseSideSubjectBuilder.ts | 2 +- .../RawSqlResultsToEntityTransformer.ts | 2 +- src/util/OrmUtils.ts | 20 +++++++++++++++ 7 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/metadata/EntityMetadata.ts b/src/metadata/EntityMetadata.ts index d91d32ca06..5b7c60fb2c 100644 --- a/src/metadata/EntityMetadata.ts +++ b/src/metadata/EntityMetadata.ts @@ -613,7 +613,7 @@ export class EntityMetadata { const secondEntityIdMap = this.getEntityIdMap(secondEntity); if (!secondEntityIdMap) return false; - return EntityMetadata.compareIds(firstEntityIdMap, secondEntityIdMap); + return OrmUtils.compareIds(firstEntityIdMap, secondEntityIdMap); } /** @@ -734,31 +734,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; - - // Optimized version for the common case - if ( - typeof firstId.id === "string" && - typeof secondId.id === "string" && - Object.keys(firstId).length === 1 && - Object.keys(secondId).length === 1 - ) { - return firstId.id === secondId.id; - } - - 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 9a55c0386d..2f2dc15ea8 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"; /** @@ -167,7 +166,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 850af6ef39..7d896cdc68 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 25250dfc5a..2578bf1cdb 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 f421c5abc7..6912768fdd 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 c1d0d670c0..1d3bde4b3e 100644 --- a/src/query-builder/transformer/RawSqlResultsToEntityTransformer.ts +++ b/src/query-builder/transformer/RawSqlResultsToEntityTransformer.ts @@ -214,7 +214,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 1691567d39..63aa9c7c10 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -121,6 +121,26 @@ export class OrmUtils { return true; } + /** + * Check if two entity-id-maps are the same + */ + static compareIds(firstId: ObjectLiteral|undefined, secondId: ObjectLiteral|undefined) { + 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" && + 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. */ From 7adc0ab9bc25505402feb9ebd2891d61482677ff Mon Sep 17 00:00:00 2001 From: Reto Kaiser Date: Sun, 23 Feb 2020 18:48:31 +0100 Subject: [PATCH 3/5] Use optimized path in compareIds() also when .id has type "number" --- src/util/OrmUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts index 63aa9c7c10..6af1d63cd1 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -130,8 +130,8 @@ export class OrmUtils { // Optimized version for the common case if ( - typeof firstId.id === "string" && - typeof secondId.id === "string" && + (typeof firstId.id === "string" || typeof firstId.id === "number") && + (typeof secondId.id === "string" || typeof secondId.id === "number") && Object.keys(firstId).length === 1 && Object.keys(secondId).length === 1 ) { From f37d231629fb516056f958e96504861e78eefe54 Mon Sep 17 00:00:00 2001 From: Reto Kaiser Date: Sun, 23 Feb 2020 18:52:23 +0100 Subject: [PATCH 4/5] Add return type to signature of deepCompare() and compareIds() --- src/util/OrmUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts index 6af1d63cd1..deb462077d 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -100,7 +100,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) { @@ -124,7 +124,7 @@ export class OrmUtils { /** * Check if two entity-id-maps are the same */ - static compareIds(firstId: ObjectLiteral|undefined, secondId: ObjectLiteral|undefined) { + static compareIds(firstId: ObjectLiteral|undefined, secondId: ObjectLiteral|undefined): boolean { if (firstId === undefined || firstId === null || secondId === undefined || secondId === null) return false; From 44d4252bdf3a707d318b4858f49f45ea28b3bb93 Mon Sep 17 00:00:00 2001 From: Umed Khudoiberdiev Date: Tue, 25 Feb 2020 19:15:11 +0500 Subject: [PATCH 5/5] made a proper check --- src/util/OrmUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts index deb462077d..3df26494cb 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -130,8 +130,8 @@ export class OrmUtils { // Optimized version for the common case if ( - (typeof firstId.id === "string" || typeof firstId.id === "number") && - (typeof secondId.id === "string" || typeof secondId.id === "number") && + ((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 ) {