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

perf: Optimized version of EntityMetadata#compareIds() for the common case #5419

Merged
merged 5 commits into from May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 2 additions & 13 deletions src/metadata/EntityMetadata.ts
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -734,21 +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;

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.
Expand Down
3 changes: 1 addition & 2 deletions 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";

/**
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions src/persistence/subject-builder/ManyToManySubjectBuilder.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/persistence/subject-builder/OneToManySubjectBuilder.ts
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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[];
Expand Down
22 changes: 21 additions & 1 deletion src/util/OrmUtils.ts
Expand Up @@ -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) {
Expand All @@ -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): 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.
*/
Expand Down