Skip to content

Commit

Permalink
perf(core): use Set instead of array for stacks in UoW
Browse files Browse the repository at this point in the history 
Related: #732
  • Loading branch information
@B4nan
B4nan committed Aug 11, 2020
1 parent bfeb2e3 commit 12ba811
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 58 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/unit-of-work/ChangeSetComputer.ts
View file
Expand Up @@ -11,7 +11,7 @@ export class ChangeSetComputer {
private readonly originalEntityData: Dictionary<EntityData<AnyEntity>>,
private readonly identifierMap: Dictionary<EntityIdentifier>,
private readonly collectionUpdates: Collection<AnyEntity>[],
private readonly removeStack: AnyEntity[],
private readonly removeStack: Set<AnyEntity>,
private readonly metadata: MetadataStorage,
private readonly platform: Platform) { }

Expand Down Expand Up @@ -57,7 +57,7 @@ export class ChangeSetComputer {
// remove items from collection based on removeStack
if (Utils.isCollection<T>(target) && target.isInitialized()) {
target.getItems()
.filter(item => this.removeStack.includes(item))
.filter(item => this.removeStack.has(item))
.forEach(item => target.remove(item));
}

Expand Down
74 changes: 31 additions & 43 deletions packages/core/src/unit-of-work/UnitOfWork.ts
View file
Expand Up @@ -18,12 +18,12 @@ export class UnitOfWork {
/** 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 persistStack: AnyEntity[] = [];
private readonly removeStack: AnyEntity[] = [];
private readonly orphanRemoveStack: AnyEntity[] = [];
private readonly persistStack = new Set<AnyEntity>();
private readonly removeStack = new Set<AnyEntity>();
private readonly orphanRemoveStack = new Set<AnyEntity>();
private readonly changeSets: ChangeSet<AnyEntity>[] = [];
private readonly collectionUpdates: Collection<AnyEntity>[] = [];
private readonly extraUpdates: [AnyEntity, string, AnyEntity | Reference<AnyEntity>][] = [];
private readonly extraUpdates = new Set<[AnyEntity, string, AnyEntity | Reference<AnyEntity>]>();
private readonly metadata = this.em.getMetadata();
private readonly platform = this.em.getDriver().getPlatform();
private readonly changeSetComputer = new ChangeSetComputer(this.em.getValidator(), this.originalEntityData, this.identifierMap, this.collectionUpdates, this.removeStack, this.metadata, this.platform);
Expand Down Expand Up @@ -79,11 +79,11 @@ export class UnitOfWork {
return this.originalEntityData;
}

getPersistStack(): AnyEntity[] {
getPersistStack(): Set<AnyEntity> {
return this.persistStack;
}

getRemoveStack(): AnyEntity[] {
getRemoveStack(): Set<AnyEntity> {
return this.removeStack;
}

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

Expand All @@ -122,33 +122,33 @@ export class UnitOfWork {
}

persist<T extends AnyEntity<T>>(entity: T, visited: AnyEntity[] = [], checkRemoveStack = false): void {
if (this.persistStack.includes(entity)) {
if (this.persistStack.has(entity)) {
return;
}

if (checkRemoveStack && this.removeStack.includes(entity)) {
if (checkRemoveStack && this.removeStack.has(entity)) {
return;
}

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

this.persistStack.push(entity);
this.cleanUpStack(this.removeStack, entity);
this.persistStack.add(entity);
this.removeStack.delete(entity);
this.cascade(entity, Cascade.PERSIST, visited, { checkRemoveStack });
}

remove(entity: AnyEntity, visited: AnyEntity[] = []): void {
if (this.removeStack.includes(entity)) {
if (this.removeStack.has(entity)) {
return;
}

if (wrap(entity, true).__primaryKey) {
this.removeStack.push(entity);
this.removeStack.add(entity);
}

this.cleanUpStack(this.persistStack, entity);
this.persistStack.delete(entity);
this.unsetIdentity(entity);
this.cascade(entity, Cascade.REMOVE, visited);
}
Expand All @@ -164,7 +164,7 @@ export class UnitOfWork {
await this.em.getEventManager().dispatchEvent(EventType.onFlush, { em: this.em, uow: this });

// nothing to do, do not start transaction
if (this.changeSets.length === 0 && this.collectionUpdates.length === 0 && this.extraUpdates.length === 0) {
if (this.changeSets.length === 0 && this.collectionUpdates.length === 0 && this.extraUpdates.size === 0) {
await this.em.getEventManager().dispatchEvent(EventType.afterFlush, { em: this.em, uow: this });
this.postCommitCleanup();

Expand Down Expand Up @@ -217,14 +217,14 @@ export class UnitOfWork {
this.changeSets.length = 0;

Object.values(this.identityMap)
.filter(entity => !this.removeStack.includes(entity) && !this.orphanRemoveStack.includes(entity))
.filter(entity => !this.removeStack.has(entity) && !this.orphanRemoveStack.has(entity))
.forEach(entity => this.persist(entity, [], true));

while (this.persistStack.length) {
this.findNewEntities(this.persistStack.shift()!);
for (const entity of this.persistStack) {
this.findNewEntities(entity);
}

for (const entity of Object.values(this.orphanRemoveStack)) {
for (const entity of this.orphanRemoveStack) {
this.remove(entity);
}

Expand All @@ -235,11 +235,11 @@ export class UnitOfWork {
}

scheduleOrphanRemoval(entity: AnyEntity): void {
this.orphanRemoveStack.push(entity);
this.orphanRemoveStack.add(entity);
}

cancelOrphanRemoval(entity: AnyEntity): void {
this.cleanUpStack(this.orphanRemoveStack, entity);
this.orphanRemoveStack.delete(entity);
}

private findNewEntities<T extends AnyEntity<T>>(entity: T, visited: AnyEntity[] = []): void {
Expand All @@ -250,7 +250,7 @@ export class UnitOfWork {
visited.push(entity);
const wrapped = wrap(entity, true);

if (!wrapped.isInitialized() || this.removeStack.includes(entity) || this.orphanRemoveStack.includes(entity)) {
if (!wrapped.isInitialized() || this.removeStack.has(entity) || this.orphanRemoveStack.has(entity)) {
return;
}

Expand All @@ -265,7 +265,7 @@ export class UnitOfWork {

if (changeSet) {
this.changeSets.push(changeSet);
this.cleanUpStack(this.persistStack, entity);
this.persistStack.delete(entity);
this.originalEntityData[wrapped.__uuid] = Utils.prepareEntity(entity, this.metadata, this.platform);
}
}
Expand Down Expand Up @@ -300,7 +300,7 @@ export class UnitOfWork {

private processToManyReference<T extends AnyEntity<T>>(reference: Collection<AnyEntity>, visited: AnyEntity[], parent: T, prop: EntityProperty<T>): void {
if (this.isCollectionSelfReferenced(reference, visited)) {
this.extraUpdates.push([parent, prop.name, reference]);
this.extraUpdates.add([parent, prop.name, reference]);
parent[prop.name as keyof T] = new Collection<AnyEntity>(parent) as unknown as T[keyof T];

return;
Expand All @@ -321,7 +321,7 @@ export class UnitOfWork {
const isScheduledForInsert = cs && cs.type === ChangeSetType.CREATE && !cs.persisted;

if (isScheduledForInsert) {
this.extraUpdates.push([changeSet.entity, prop.name, changeSet.entity[prop.name]]);
this.extraUpdates.add([changeSet.entity, prop.name, changeSet.entity[prop.name]]);
delete changeSet.entity[prop.name];
delete changeSet.payload[prop.name];
}
Expand All @@ -347,25 +347,14 @@ export class UnitOfWork {
await this.em.getEventManager().dispatchEvent(type, { entity: changeSet.entity, em: this.em, changeSet });
}

/**
* clean up persist/remove stack from previous persist/remove calls for this entity done before flushing
*/
private cleanUpStack(stack: AnyEntity[], entity: AnyEntity): void {
const idx = stack.indexOf(entity);

if (idx !== -1) {
stack.splice(idx, 1);
}
}

private postCommitCleanup(): void {
Object.keys(this.identifierMap).forEach(key => delete this.identifierMap[key]);
this.persistStack.length = 0;
this.removeStack.length = 0;
this.orphanRemoveStack.length = 0;
this.persistStack.clear();
this.removeStack.clear();
this.orphanRemoveStack.clear();
this.changeSets.length = 0;
this.collectionUpdates.length = 0;
this.extraUpdates.length = 0;
this.extraUpdates.clear();
this.working = false;
}

Expand Down Expand Up @@ -485,8 +474,7 @@ export class UnitOfWork {
await this.commitChangeSet(changeSet, tx);
}

while (this.extraUpdates.length) {
const extraUpdate = this.extraUpdates.shift()!;
for (const extraUpdate of this.extraUpdates) {
extraUpdate[0][extraUpdate[1]] = extraUpdate[2];
const changeSet = this.changeSetComputer.computeChangeSet(extraUpdate[0])!;

Expand Down Expand Up @@ -516,7 +504,7 @@ export class UnitOfWork {
return arr.indexOf(a[key]) - arr.indexOf(b[key]);
};

const copy = this.changeSets.slice(); // make copy to maintain commitOrder
const copy = this.changeSets.slice(); // make copy to maintain commit order
this.changeSets.sort((a, b) => {
if (a.type !== b.type) {
return compare(copy, typeOrder, a, b, 'type');
Expand Down
21 changes: 8 additions & 13 deletions tests/UnitOfWork.test.ts
View file
Expand Up @@ -97,35 +97,30 @@ describe('UnitOfWork', () => {
const author = new Author('test', 'test');
author.id = '00000001885f0a3cc37dc9f0';
uow.persist(author);
// @ts-ignore
expect(uow.persistStack.length).toBe(1);
expect(uow.getPersistStack().size).toBe(1);
uow.persist(author);
// @ts-ignore
expect(uow.persistStack.length).toBe(1);
expect(uow.getPersistStack().size).toBe(1);
uow.remove(author);
// @ts-ignore
expect(uow.persistStack.length).toBe(0);
// @ts-ignore
expect(uow.removeStack.length).toBe(1);
expect(uow.getPersistStack().size).toBe(0);
expect(uow.getRemoveStack().size).toBe(1);
uow.remove(author);
// @ts-ignore
expect(uow.removeStack.length).toBe(1);
expect(uow.getRemoveStack().size).toBe(1);
});

test('getters', async () => {
const uow = new UnitOfWork(orm.em);
const author = new Author('test', 'test');
author.id = '00000001885f0a3cc37dc9f0';
uow.persist(author);
expect(uow.getPersistStack()).toEqual([author]);
expect(uow.getRemoveStack()).toEqual([]);
expect([...uow.getPersistStack()]).toEqual([author]);
expect([...uow.getRemoveStack()]).toEqual([]);
expect(uow.getOriginalEntityData()).toEqual({});
uow.merge(author);
expect(uow.getOriginalEntityData()).toMatchObject({
[wrap(author, true).__uuid]: { name: 'test', email: 'test' },
});
uow.remove(author);
expect(uow.getRemoveStack()).toEqual([author]);
expect([...uow.getRemoveStack()]).toEqual([author]);
expect(() => uow.recomputeSingleChangeSet(author)).not.toThrow();
expect(() => uow.computeChangeSet(author)).not.toThrow();
expect(() => uow.recomputeSingleChangeSet(author)).not.toThrow();
Expand Down

0 comments on commit 12ba811

Please sign in to comment.