Skip to content

Commit

Permalink
perf(core): use actual Map for identity maps
Browse files Browse the repository at this point in the history
Related: #732
  • Loading branch information
B4nan committed Aug 13, 2020
1 parent 66ffc3b commit 3645a20
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 57 deletions.
4 changes: 3 additions & 1 deletion packages/core/src/EntityManager.ts
Expand Up @@ -613,7 +613,9 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
em.filterParams = Utils.copy(this.filterParams);

if (!clear) {
Object.values(this.getUnitOfWork().getIdentityMap()).forEach(entity => em.merge(entity));
for (const entity of this.getUnitOfWork().getIdentityMap().values()) {
em.merge(entity);
}
}

return em;
Expand Down
18 changes: 9 additions & 9 deletions packages/core/src/unit-of-work/ChangeSetComputer.ts
@@ -1,15 +1,15 @@
import { Utils } from '../utils';
import { MetadataStorage } from '../metadata';
import { AnyEntity, Dictionary, EntityData, EntityProperty, Primary } from '../typings';
import { AnyEntity, EntityData, EntityProperty, Primary } from '../typings';
import { ChangeSet, ChangeSetType } from './ChangeSet';
import { Collection, EntityIdentifier, EntityValidator, ReferenceType } from '../entity';
import { Platform } from '../platforms';

export class ChangeSetComputer {

constructor(private readonly validator: EntityValidator,
private readonly originalEntityData: Dictionary<EntityData<AnyEntity>>,
private readonly identifierMap: Dictionary<EntityIdentifier>,
private readonly originalEntityData: Map<string, EntityData<AnyEntity>>,
private readonly identifierMap: Map<string, EntityIdentifier>,
private readonly collectionUpdates: Collection<AnyEntity>[],
private readonly removeStack: Set<AnyEntity>,
private readonly metadata: MetadataStorage,
Expand All @@ -24,12 +24,12 @@ export class ChangeSetComputer {
}

changeSet.name = meta.name;
changeSet.type = this.originalEntityData[entity.__helper!.__uuid] ? ChangeSetType.UPDATE : ChangeSetType.CREATE;
changeSet.type = this.originalEntityData.has(entity.__helper!.__uuid) ? ChangeSetType.UPDATE : ChangeSetType.CREATE;
changeSet.collection = meta.collection;
changeSet.payload = this.computePayload(entity);

if (changeSet.type === ChangeSetType.UPDATE) {
changeSet.originalEntity = this.originalEntityData[entity.__helper!.__uuid];
changeSet.originalEntity = this.originalEntityData.get(entity.__helper!.__uuid);
}

this.validator.validate<T>(changeSet.entity, changeSet.payload, meta);
Expand All @@ -46,8 +46,8 @@ export class ChangeSetComputer {
}

private computePayload<T extends AnyEntity<T>>(entity: T): EntityData<T> {
if (this.originalEntityData[entity.__helper!.__uuid]) {
return Utils.diffEntities<T>(this.originalEntityData[entity.__helper!.__uuid] as T, entity, this.metadata, this.platform);
if (this.originalEntityData.get(entity.__helper!.__uuid)) {
return Utils.diffEntities<T>(this.originalEntityData.get(entity.__helper!.__uuid) as T, entity, this.metadata, this.platform);
}

return Utils.prepareEntity(entity, this.metadata, this.platform);
Expand Down Expand Up @@ -80,7 +80,7 @@ export class ChangeSetComputer {
const isToOneOwner = prop.reference === ReferenceType.MANY_TO_ONE || (prop.reference === ReferenceType.ONE_TO_ONE && prop.owner);

if (isToOneOwner && pks.length === 1 && !Utils.isDefined(entity[pks[0]], true)) {
changeSet.payload[prop.name] = this.identifierMap[entity.__helper!.__uuid];
changeSet.payload[prop.name] = this.identifierMap.get(entity.__helper!.__uuid);
}
}

Expand All @@ -100,7 +100,7 @@ export class ChangeSetComputer {

private processOneToOne<T extends AnyEntity<T>>(prop: EntityProperty<T>, changeSet: ChangeSet<T>): void {
// check diff, if we had a value on 1:1 before and now it changed (nulled or replaced), we need to trigger orphan removal
const data = this.originalEntityData[changeSet.entity.__helper!.__uuid] as EntityData<T>;
const data = this.originalEntityData.get(changeSet.entity.__helper!.__uuid) as EntityData<T>;
const em = changeSet.entity.__helper!.__em;

if (prop.orphanRemoval && data && data[prop.name] && prop.name in changeSet.payload && em) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/unit-of-work/ChangeSetPersister.ts
Expand Up @@ -10,7 +10,7 @@ import { Hydrator } from '../hydration';
export class ChangeSetPersister {

constructor(private readonly driver: IDatabaseDriver,
private readonly identifierMap: Dictionary<EntityIdentifier>,
private readonly identifierMap: Map<string, EntityIdentifier>,
private readonly metadata: MetadataStorage,
private readonly hydrator: Hydrator) { }

Expand Down Expand Up @@ -55,7 +55,7 @@ export class ChangeSetPersister {
const insertId = prop.customType ? prop.customType.convertToJSValue(value, this.driver.getPlatform()) : value;
const wrapped = changeSet.entity.__helper!;
wrapped.__primaryKey = Utils.isDefined(wrapped.__primaryKey, true) ? wrapped.__primaryKey : insertId;
this.identifierMap[wrapped.__uuid].setValue(changeSet.entity[prop.name] as unknown as IPrimaryKey);
this.identifierMap.get(wrapped.__uuid)!.setValue(changeSet.entity[prop.name] as unknown as IPrimaryKey);
}

private async updateEntity<T extends AnyEntity<T>>(meta: EntityMetadata<T>, changeSet: ChangeSet<T>, ctx?: Transaction): Promise<QueryResult> {
Expand Down
58 changes: 30 additions & 28 deletions packages/core/src/unit-of-work/UnitOfWork.ts
@@ -1,4 +1,4 @@
import { AnyEntity, Dictionary, EntityData, EntityMetadata, EntityProperty, FilterQuery, Primary } from '../typings';
import { AnyEntity, EntityData, EntityMetadata, EntityProperty, FilterQuery, Primary } from '../typings';
import { Cascade, Collection, EntityIdentifier, Reference, ReferenceType } from '../entity';
import { ChangeSet, ChangeSetType } from './ChangeSet';
import { ChangeSetComputer, ChangeSetPersister, CommitOrderCalculator } from './index';
Expand All @@ -10,13 +10,13 @@ import { Transaction } from '../connections';
export class UnitOfWork {

/** map of references to managed entities */
private readonly identityMap = Object.create(null) as Dictionary<AnyEntity>;
private readonly identityMap = new Map<string, AnyEntity>();

/** holds copy of identity map so we can compute changes when persisting managed entities */
private readonly originalEntityData = Object.create(null) as Dictionary<EntityData<AnyEntity>>;
private readonly originalEntityData = new Map<string, EntityData<AnyEntity>>();

/** map of wrapped primary keys so we can compute change set without eager commit */
private readonly identifierMap = Object.create(null) as Dictionary<EntityIdentifier>;
private readonly identifierMap = new Map<string, EntityIdentifier>();

private readonly persistStack = new Set<AnyEntity>();
private readonly removeStack = new Set<AnyEntity>();
Expand All @@ -41,10 +41,10 @@ export class UnitOfWork {
}

const root = Utils.getRootEntity(this.metadata, wrapped.__meta);
this.identityMap[`${root.name}-${wrapped.__serializedPrimaryKey}`] = entity;
this.identityMap.set(`${root.name}-${wrapped.__serializedPrimaryKey}`, entity);

if (mergeData || !this.originalEntityData[entity.__helper!.__uuid]) {
this.originalEntityData[entity.__helper!.__uuid] = Utils.prepareEntity(entity, this.metadata, this.platform);
if (mergeData || !this.originalEntityData.has(entity.__helper!.__uuid)) {
this.originalEntityData.set(entity.__helper!.__uuid, Utils.prepareEntity(entity, this.metadata, this.platform));
}

this.cascade(entity, Cascade.MERGE, visited, { mergeData: false });
Expand All @@ -58,7 +58,7 @@ export class UnitOfWork {
const hash = Utils.getPrimaryKeyHash(Utils.asArray(id) as string[]);
const token = `${root.name}-${hash}`;

return this.identityMap[token] as T;
return this.identityMap.get(token) as T;
}

tryGetById<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, strict = true): T | null {
Expand All @@ -71,11 +71,11 @@ export class UnitOfWork {
return this.getById<T>(entityName, pk);
}

getIdentityMap(): Dictionary<AnyEntity> {
getIdentityMap(): Map<string, AnyEntity> {
return this.identityMap;
}

getOriginalEntityData(): Dictionary<EntityData<AnyEntity>> {
getOriginalEntityData(): Map<string, EntityData<AnyEntity>> {
return this.originalEntityData;
}

Expand All @@ -101,7 +101,7 @@ export class UnitOfWork {
this.initIdentifier(entity);
this.changeSets.push(cs);
this.persistStack.delete(entity);
this.originalEntityData[entity.__helper!.__uuid] = Utils.prepareEntity(entity, this.metadata, this.platform);
this.originalEntityData.set(entity.__helper!.__uuid, Utils.prepareEntity(entity, this.metadata, this.platform));
}

recomputeSingleChangeSet<T extends AnyEntity<T>>(entity: T): void {
Expand All @@ -115,7 +115,7 @@ export class UnitOfWork {

if (cs) {
Object.assign(this.changeSets[idx].payload, cs.payload);
this.originalEntityData[entity.__helper!.__uuid] = Utils.prepareEntity(entity, this.metadata, this.platform);
this.originalEntityData.set(entity.__helper!.__uuid, Utils.prepareEntity(entity, this.metadata, this.platform));
}
}

Expand All @@ -129,7 +129,7 @@ export class UnitOfWork {
}

if (!Utils.isDefined(entity.__helper!.__primaryKey, true)) {
this.identifierMap[entity.__helper!.__uuid] = new EntityIdentifier();
this.identifierMap.set(entity.__helper!.__uuid, new EntityIdentifier());
}

this.persistStack.add(entity);
Expand Down Expand Up @@ -198,25 +198,27 @@ export class UnitOfWork {
}

clear(): void {
Object.keys(this.identityMap).forEach(key => delete this.identityMap[key]);
Object.keys(this.originalEntityData).forEach(key => delete this.originalEntityData[key]);
this.identityMap.clear();
this.originalEntityData.clear();
this.postCommitCleanup();
}

unsetIdentity(entity: AnyEntity): void {
const wrapped = entity.__helper!;
const root = Utils.getRootEntity(this.metadata, wrapped.__meta);
delete this.identityMap[`${root.name}-${wrapped.__serializedPrimaryKey}`];
delete this.identifierMap[wrapped.__uuid];
delete this.originalEntityData[wrapped.__uuid];
this.identityMap.delete(`${root.name}-${wrapped.__serializedPrimaryKey}`);
this.identifierMap.delete(wrapped.__uuid);
this.originalEntityData.delete(wrapped.__uuid);
}

computeChangeSets(): void {
this.changeSets.length = 0;

Object.values(this.identityMap)
.filter(entity => !this.removeStack.has(entity) && !this.orphanRemoveStack.has(entity))
.forEach(entity => this.persist(entity, undefined, true));
for (const entity of this.identityMap.values()) {
if (!this.removeStack.has(entity) && !this.orphanRemoveStack.has(entity)) {
this.persist(entity, undefined, true);
}
}

for (const entity of this.persistStack) {
this.findNewEntities(entity);
Expand Down Expand Up @@ -264,18 +266,18 @@ export class UnitOfWork {
if (changeSet) {
this.changeSets.push(changeSet);
this.persistStack.delete(entity);
this.originalEntityData[wrapped.__uuid] = Utils.prepareEntity(entity, this.metadata, this.platform);
this.originalEntityData.set(wrapped.__uuid, Utils.prepareEntity(entity, this.metadata, this.platform));
}
}

private initIdentifier<T extends AnyEntity<T>>(entity: T): void {
const wrapped = entity.__helper!;

if (Utils.isDefined(wrapped.__primaryKey, true) || this.identifierMap[wrapped.__uuid]) {
if (Utils.isDefined(wrapped.__primaryKey, true) || this.identifierMap.has(wrapped.__uuid)) {
return;
}

this.identifierMap[wrapped.__uuid] = new EntityIdentifier();
this.identifierMap.set(wrapped.__uuid, new EntityIdentifier());
}

private processReference<T extends AnyEntity<T>>(parent: T, prop: EntityProperty<T>, reference: any, visited: Set<AnyEntity>): void {
Expand All @@ -291,7 +293,7 @@ export class UnitOfWork {
}

private processToOneReference<T extends AnyEntity<T>>(reference: any, visited: Set<AnyEntity>): void {
if (!this.originalEntityData[reference.__helper!.__uuid]) {
if (!this.originalEntityData.has(reference.__helper!.__uuid)) {
this.findNewEntities(reference, visited);
}
}
Expand All @@ -305,7 +307,7 @@ export class UnitOfWork {
}

reference.getItems(false)
.filter(item => !this.originalEntityData[item.__helper!.__uuid])
.filter(item => !this.originalEntityData.has(item.__helper!.__uuid))
.forEach(item => this.findNewEntities(item, visited));
}

Expand Down Expand Up @@ -346,7 +348,7 @@ export class UnitOfWork {
}

private postCommitCleanup(): void {
Object.keys(this.identifierMap).forEach(key => delete this.identifierMap[key]);
this.identifierMap.clear();
this.persistStack.clear();
this.removeStack.clear();
this.orphanRemoveStack.clear();
Expand Down Expand Up @@ -401,7 +403,7 @@ export class UnitOfWork {
}

private isCollectionSelfReferenced(collection: Collection<AnyEntity>, visited: Set<AnyEntity>): boolean {
const filtered = collection.getItems(false).filter(item => !this.originalEntityData[item.__helper!.__uuid]);
const filtered = collection.getItems(false).filter(item => !this.originalEntityData.has(item.__helper!.__uuid));
return filtered.some(items => visited.has(items));
}

Expand Down
16 changes: 7 additions & 9 deletions tests/EntityManager.mongo.test.ts
Expand Up @@ -301,7 +301,7 @@ describe('EntityManagerMongo', () => {
repo.persist(author);
repo.persist(author2);
await repo.removeAndFlush(author);
expect(Object.keys(orm.em.getUnitOfWork().getIdentityMap())).toEqual([`Author-${author2.id}`]);
expect([...orm.em.getUnitOfWork().getIdentityMap().keys()]).toEqual([`Author-${author2.id}`]);
author2.name = 'lol';
repo.persist(author2);
orm.em.removeLater(author3);
Expand All @@ -317,7 +317,7 @@ describe('EntityManagerMongo', () => {
repo.persist(author);
orm.em.remove(author);
expect(orm.em.getUnitOfWork().getById<Author>(Author.name, author.id)).toBeUndefined();
expect(orm.em.getUnitOfWork().getIdentityMap()).toEqual({});
expect(orm.em.getUnitOfWork().getIdentityMap()).toEqual(new Map());
});

test('removing persisted entity via PK', async () => {
Expand Down Expand Up @@ -383,7 +383,7 @@ describe('EntityManagerMongo', () => {

expect(fork).not.toBe(orm.em);
expect(fork.getMetadata()).toBe(orm.em.getMetadata());
expect(fork.getUnitOfWork().getIdentityMap()).toEqual({});
expect(fork.getUnitOfWork().getIdentityMap()).toEqual(new Map());

// request context is not started so we can use UoW and EF getters
expect(fork.getUnitOfWork().getIdentityMap()).not.toBe(orm.em.getUnitOfWork().getIdentityMap());
Expand Down Expand Up @@ -818,7 +818,7 @@ describe('EntityManagerMongo', () => {
orm.em.clear();
const cachedAuthor = orm.em.merge<Author>(Author, cache);
expect(cachedAuthor).toBe(cachedAuthor.favouriteBook.author);
expect(Object.keys(orm.em.getUnitOfWork().getIdentityMap())).toEqual([
expect([...orm.em.getUnitOfWork().getIdentityMap().keys()]).toEqual([
'Author-' + author.id,
'Book-' + book1.id,
'BookTag-' + tag1.id,
Expand All @@ -833,7 +833,7 @@ describe('EntityManagerMongo', () => {
orm.em.clear();
const cachedAuthor2 = orm.em.merge(author);
expect(cachedAuthor2).toBe(cachedAuthor2.favouriteBook.author);
expect(Object.keys(orm.em.getUnitOfWork().getIdentityMap())).toEqual([
expect([...orm.em.getUnitOfWork().getIdentityMap().keys()]).toEqual([
'Author-' + author.id,
'Book-' + book1.id,
'BookTag-' + tag1.id,
Expand Down Expand Up @@ -1530,14 +1530,12 @@ describe('EntityManagerMongo', () => {
const baz2 = FooBaz.create('fz2');
bar.baz = baz1;
await orm.em.persistAndFlush(bar);
// @ts-ignore
expect(orm.em.getUnitOfWork().originalEntityData[wrap(bar, true).__uuid].baz).toEqual(baz1._id);
expect(orm.em.getUnitOfWork().getOriginalEntityData().get(wrap(bar, true).__uuid)!.baz).toEqual(baz1._id);

// replacing reference with value will trigger orphan removal
bar.baz = baz2;
await orm.em.persistAndFlush(bar);
// @ts-ignore
expect(orm.em.getUnitOfWork().originalEntityData[wrap(bar, true).__uuid].baz).toEqual(baz2._id);
expect(orm.em.getUnitOfWork().getOriginalEntityData().get(wrap(bar, true).__uuid)!.baz).toEqual(baz2._id);
await expect(orm.em.findOne(FooBaz, baz1)).resolves.toBeNull();
await expect(orm.em.findOne(FooBaz, baz2)).resolves.not.toBeNull();

Expand Down
10 changes: 4 additions & 6 deletions tests/UnitOfWork.test.ts
Expand Up @@ -89,9 +89,9 @@ describe('UnitOfWork', () => {
uow.merge(author); // add entity to IM first
const changeSet = await computer.computeChangeSet(author); // then try to persist it again
expect(changeSet).toBeNull();
expect(uow.getIdentityMap()).not.toEqual({});
expect(uow.getIdentityMap()).not.toEqual(new Map());
uow.clear();
expect(uow.getIdentityMap()).toEqual({});
expect(uow.getIdentityMap()).toEqual(new Map());
});

test('changeSet is null for readonly entity', async () => {
Expand Down Expand Up @@ -122,11 +122,9 @@ describe('UnitOfWork', () => {
uow.persist(author);
expect([...uow.getPersistStack()]).toEqual([author]);
expect([...uow.getRemoveStack()]).toEqual([]);
expect(uow.getOriginalEntityData()).toEqual({});
expect(uow.getOriginalEntityData()).toEqual(new Map());
uow.merge(author);
expect(uow.getOriginalEntityData()).toMatchObject({
[wrap(author, true).__uuid]: { name: 'test', email: 'test' },
});
expect(uow.getOriginalEntityData().get(wrap(author, true).__uuid)).toMatchObject({ name: 'test', email: 'test' });
uow.remove(author);
expect([...uow.getRemoveStack()]).toEqual([author]);
expect(() => uow.recomputeSingleChangeSet(author)).not.toThrow();
Expand Down
2 changes: 1 addition & 1 deletion tests/composite-keys.mysql.test.ts
Expand Up @@ -52,7 +52,7 @@ describe('composite keys in mysql', () => {
expect(p2.bar.id).toBe(bar.id);
expect(p2.baz.id).toBe(baz.id);
expect(p2.value).toBe('val2');
expect(Object.keys(orm.em.getUnitOfWork().getIdentityMap()).sort()).toEqual(['FooBar2-7', 'FooBaz2-3', 'FooParam2-7~~~3']);
expect([...orm.em.getUnitOfWork().getIdentityMap().keys()].sort()).toEqual(['FooBar2-7', 'FooBaz2-3', 'FooParam2-7~~~3']);

const p3 = await orm.em.findOneOrFail(FooParam2, { bar: param.bar.id, baz: param.baz.id });
expect(p3).toBe(p2);
Expand Down
2 changes: 1 addition & 1 deletion tests/single-table-inheritance.mysql.test.ts
Expand Up @@ -91,7 +91,7 @@ describe('single table inheritance in mysql', () => {
favouriteManager: users[2],
});

expect(Object.keys(orm.em.getUnitOfWork().getIdentityMap())).toEqual(['BaseUser2-2', 'BaseUser2-1', 'BaseUser2-3', 'BaseUser2-4']);
expect([...orm.em.getUnitOfWork().getIdentityMap().keys()]).toEqual(['BaseUser2-2', 'BaseUser2-1', 'BaseUser2-3', 'BaseUser2-4']);

const o = await orm.em.findOneOrFail(CompanyOwner2, 4);
expect(o.state).toBeUndefined();
Expand Down

0 comments on commit 3645a20

Please sign in to comment.