Skip to content

Commit

Permalink
perf(core): remove WrappedEntity.__internal map
Browse files Browse the repository at this point in the history
It's probably better not to create additional object for each entity.
  • Loading branch information
B4nan committed Sep 15, 2020
1 parent f4f90eb commit 2228fcb
Show file tree
Hide file tree
Showing 19 changed files with 39 additions and 58 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/EntityManager.ts
Expand Up @@ -222,7 +222,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
let entity = this.getUnitOfWork().tryGetById<T>(entityName, where);
const isOptimisticLocking = !Utils.isDefined(options.lockMode) || options.lockMode === LockMode.OPTIMISTIC;

if (entity && entity.__helper!.isInitialized() && !options.refresh && isOptimisticLocking) {
if (entity && entity.__helper!.__initialized && !options.refresh && isOptimisticLocking) {
return this.lockAndPopulate<T, P>(entityName, entity, where, options);
}

Expand Down Expand Up @@ -419,7 +419,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
this.validator.validatePrimaryKey(data as EntityData<T>, this.metadata.get(entityName));
let entity = this.getUnitOfWork().tryGetById<T>(entityName, data as FilterQuery<T>, false);

if (entity && entity.__helper!.isInitialized() && !refresh) {
if (entity && entity.__helper!.__initialized && !refresh) {
return entity;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/ArrayCollection.ts
Expand Up @@ -114,7 +114,7 @@ export class ArrayCollection<T, O> {

isInitialized(fully = false): boolean {
if (fully) {
return this.initialized && this.items.every((item: AnyEntity<T>) => item.__helper!.isInitialized());
return this.initialized && this.items.every((item: AnyEntity<T>) => item.__helper!.__initialized);
}

return this.initialized;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/BaseEntity.ts
Expand Up @@ -9,7 +9,7 @@ export abstract class BaseEntity<T extends AnyEntity<T>, PK extends keyof T> imp
}

isInitialized(): boolean {
return (this as unknown as T).__helper!.isInitialized();
return (this as unknown as T).__helper!.__initialized;
}

populated(populated = true): void {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/entity/Collection.ts
Expand Up @@ -230,7 +230,7 @@ export class Collection<T, O = unknown> extends ArrayCollection<T, O> {
}

private createManyToManyCondition(cond: Dictionary) {
if (this.property.owner || this.owner.__helper!.__internal.platform.usesPivotTable()) {
if (this.property.owner || this.property.pivotTable) {
const pk = (this.items[0] as AnyEntity<T>).__helper!.__meta.primaryKeys[0]; // we know there is at least one item as it was checked in load method
cond[pk] = { $in: this.items.map((item: AnyEntity<T>) => item.__helper!.__primaryKey) };
} else {
Expand Down Expand Up @@ -283,12 +283,12 @@ export class Collection<T, O = unknown> extends ArrayCollection<T, O> {

private validateModification(items: T[]): void {
// currently we allow persisting to inverse sides only in SQL drivers
if (this.owner.__helper!.__internal.platform.usesPivotTable() || !this.property.mappedBy) {
if (this.property.pivotTable || !this.property.mappedBy) {
return;
}

const check = (item: T & AnyEntity<T>) => {
if (item.__helper!.isInitialized()) {
if (item.__helper!.__initialized) {
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/entity/EntityAssigner.ts
Expand Up @@ -15,8 +15,8 @@ export class EntityAssigner {
const wrapped = entity.__helper!;
const em = options.em || wrapped.__em;
const meta = wrapped.__meta;
const validator = wrapped.__internal.validator;
const platform = wrapped.__internal.platform;
const validator = wrapped.__internal.getValidator();
const platform = wrapped.__internal.getDriver().getPlatform();
const props = meta.properties;

Object.keys(data).forEach(prop => {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/entity/EntityFactory.ts
Expand Up @@ -37,7 +37,7 @@ export class EntityFactory {
const meta2 = this.processDiscriminatorColumn<T>(meta, data);
const exists = this.findEntity<T>(data, meta2, options.convertCustomTypes);

if (exists && exists.__helper!.isInitialized() && !options.refresh) {
if (exists && exists.__helper!.__initialized && !options.refresh) {
return exists as New<T, P>;
}

Expand Down Expand Up @@ -87,15 +87,15 @@ export class EntityFactory {
// creates new instance via constructor as this is the new entity
const entity = new Entity(...params);
// perf: create the helper instance early to bypass the double getter defined on the prototype in EntityHelper
Object.defineProperty(entity, '__helper', { value: new WrappedEntity(entity as T, this.em) });
Object.defineProperty(entity, '__helper', { value: new WrappedEntity(entity as T, this.em, meta) });

return entity;
}

// creates new entity instance, bypassing constructor call as its already persisted entity
const entity = Object.create(meta.class.prototype) as T & AnyEntity<T>;
// perf: create the helper instance early to bypass the double getter defined on the prototype in EntityHelper
Object.defineProperty(entity, '__helper', { value: new WrappedEntity(entity as T, this.em) });
Object.defineProperty(entity, '__helper', { value: new WrappedEntity(entity as T, this.em, meta) });
entity.__helper!.__managed = true;
this.hydrator.hydrateReference(entity, meta, data, options.convertCustomTypes);

Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/entity/EntityHelper.ts
Expand Up @@ -53,11 +53,10 @@ export class EntityHelper {
private static defineBaseProperties<T extends AnyEntity<T>>(meta: EntityMetadata<T>, prototype: T, em: EntityManager) {
Object.defineProperties(prototype, {
__entity: { value: true },
__meta: { value: meta },
__helper: {
get(): string {
if (!this.___helper) {
const helper = new WrappedEntity(this, em);
const helper = new WrappedEntity(this, em, meta);
Object.defineProperty(this, '___helper', { value: helper, writable: true });
}

Expand Down Expand Up @@ -94,7 +93,7 @@ export class EntityHelper {
let name = meta.name;

// distinguish not initialized entities
if (!this.__helper!.isInitialized()) {
if (!this.__helper!.__initialized) {
name = `Ref<${name}>`;
}

Expand Down Expand Up @@ -125,7 +124,7 @@ export class EntityHelper {
inverse.add(owner);
}

if (prop.reference === ReferenceType.ONE_TO_ONE && entity && entity.__helper!.isInitialized() && Reference.unwrapReference(inverse) !== owner) {
if (prop.reference === ReferenceType.ONE_TO_ONE && entity && entity.__helper!.__initialized && Reference.unwrapReference(inverse) !== owner) {
EntityHelper.propagateOneToOne(entity, owner, prop);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/EntityLoader.ts
Expand Up @@ -273,7 +273,7 @@ export class EntityLoader {
return children.map(e => Reference.unwrapReference(e[field]));
}

return children.filter(e => !(e[field] as AnyEntity).__helper!.isInitialized()).map(e => Reference.unwrapReference(e[field]));
return children.filter(e => !(e[field] as AnyEntity).__helper!.__initialized).map(e => Reference.unwrapReference(e[field]));
}

private lookupAllRelationships<T>(entityName: string, prefix = '', visited: string[] = []): PopulateOptions<T>[] {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/entity/EntityTransformer.ts
Expand Up @@ -10,7 +10,7 @@ export class EntityTransformer {

static toObject<T extends AnyEntity<T>>(entity: T, ignoreFields: string[] = [], visited = new WeakSet<AnyEntity>()): EntityData<T> {
const wrapped = entity.__helper!;
const platform = wrapped.__internal.platform;
const platform = wrapped.__internal.getDriver().getPlatform();
const meta = wrapped.__meta;
const ret = {} as EntityData<T>;

Expand Down Expand Up @@ -77,7 +77,7 @@ export class EntityTransformer {
private static processProperty<T extends AnyEntity<T>>(prop: keyof T & string, entity: T, ignoreFields: string[], visited: WeakSet<AnyEntity>): T[keyof T] | undefined {
const wrapped = entity.__helper!;
const property = wrapped.__meta.properties[prop];
const platform = wrapped.__internal.platform;
const platform = wrapped.__internal.getDriver().getPlatform();

/* istanbul ignore next */
const serializer = property?.serializer;
Expand Down Expand Up @@ -113,7 +113,7 @@ export class EntityTransformer {
return wrap(child).toJSON(...args) as T[keyof T];
}

return wrapped.__internal.platform.normalizePrimaryKey(wrapped.__primaryKey as unknown as IPrimaryKey) as unknown as T[keyof T];
return wrapped.__internal.getDriver().getPlatform().normalizePrimaryKey(wrapped.__primaryKey as unknown as IPrimaryKey) as unknown as T[keyof T];
}

private static processCollection<T extends AnyEntity<T>>(prop: keyof T, entity: T): T[keyof T] | undefined {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/EntityValidator.ts
Expand Up @@ -85,7 +85,7 @@ export class EntityValidator {
}

private validateCollection<T extends AnyEntity<T>>(entity: T, prop: EntityProperty): void {
if (entity.__helper!.isInitialized() && !entity[prop.name as keyof T]) {
if (entity.__helper!.__initialized && !entity[prop.name as keyof T]) {
throw ValidationError.fromCollectionNotInitialized(entity, prop);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/Reference.ts
Expand Up @@ -102,7 +102,7 @@ export class Reference<T extends AnyEntity<T>> {
}

isInitialized(): boolean {
return this.entity.__helper!.isInitialized();
return this.entity.__helper!.__initialized;
}

populated(populated?: boolean): void {
Expand Down
27 changes: 6 additions & 21 deletions packages/core/src/entity/WrappedEntity.ts
@@ -1,7 +1,4 @@
import { EntityManager } from '../EntityManager';
import { Platform } from '../platforms';
import { MetadataStorage } from '../metadata';
import { EntityValidator } from './EntityValidator';
import { AnyEntity, Dictionary, EntityData, EntityMetadata, Populate, Primary } from '../typings';
import { IdentifiedReference, Reference } from './Reference';
import { EntityTransformer } from './EntityTransformer';
Expand All @@ -12,11 +9,10 @@ import { ValidationError } from '../errors';

export class WrappedEntity<T extends AnyEntity<T>, PK extends keyof T> {

readonly __meta: EntityMetadata<T>;
__initialized = true;
__populated = false;
__lazyInitialized = false;
__managed = false;
__populated?: boolean;
__lazyInitialized?: boolean;
__managed?: boolean;
__em?: EntityManager;

/** holds last entity data snapshot so we can compute changes when persisting managed entities */
Expand All @@ -25,20 +21,9 @@ export class WrappedEntity<T extends AnyEntity<T>, PK extends keyof T> {
/** holds wrapped primary key so we can compute change set without eager commit */
__identifier?: EntityData<T>;

readonly __internal: {
platform: Platform;
metadata: MetadataStorage;
validator: EntityValidator;
};

constructor(private readonly entity: T, em: EntityManager) {
this.__meta = (entity as Dictionary).__meta;
this.__internal = {
platform: em.getDriver().getPlatform(),
metadata: em.getMetadata(),
validator: em.getValidator(),
};
}
constructor(private readonly entity: T,
readonly __internal: EntityManager,
readonly __meta: EntityMetadata<T>) { }

isInitialized(): boolean {
return this.__initialized;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/metadata/MetadataDiscovery.ts
Expand Up @@ -282,7 +282,7 @@ export class MetadataDiscovery {
const meta2 = this.metadata.get(prop.type);
Utils.defaultValue(prop, 'fixedOrder', !!prop.fixedOrderColumn);

if (!prop.pivotTable && prop.owner) {
if (!prop.pivotTable && prop.owner && this.platform.usesPivotTable()) {
prop.pivotTable = this.namingStrategy.joinTableName(meta.collection, meta2.collection, prop.name);
}

Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/typings.ts
@@ -1,6 +1,5 @@
import { Cascade, EventType, LoadStrategy, QueryOrder, ReferenceType, LockMode } from './enums';
import { AssignOptions, Collection, EntityRepository, EntityValidator, EntityIdentifier, IdentifiedReference, Reference } from './entity';
import { Platform } from './platforms';
import { AssignOptions, Collection, EntityRepository, EntityIdentifier, IdentifiedReference, Reference } from './entity';
import { EntitySchema } from './metadata';
import { Type } from './types';

Expand Down Expand Up @@ -72,10 +71,10 @@ export interface IWrappedEntity<T extends AnyEntity<T>, PK extends keyof T, P =

export interface IWrappedEntityInternal<T extends AnyEntity<T>, PK extends keyof T, P = keyof T> extends IWrappedEntity<T, PK, P> {
__meta: EntityMetadata<T>;
__internal: { platform: Platform; metadata: IMetadataStorage; validator: EntityValidator };
__data: Dictionary;
__em?: any; // we cannot have `EntityManager` here as that causes a cycle
__initialized?: boolean;
__internal: any; // we cannot have `EntityManager` here as that causes a cycle
__initialized: boolean;
__originalEntityData?: EntityData<T>;
__identifier?: EntityIdentifier;
__managed: boolean;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/unit-of-work/ChangeSetComputer.ts
Expand Up @@ -98,7 +98,7 @@ export class ChangeSetComputer {
return;
}

if (prop.owner || target.getItems(false).filter(item => !item.__helper!.isInitialized()).length > 0) {
if (prop.owner || target.getItems(false).filter(item => !item.__helper!.__initialized).length > 0) {
this.collectionUpdates.push(target);
} else {
target.setDirty(false); // inverse side with only populated items, nothing to persist
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/unit-of-work/UnitOfWork.ts
Expand Up @@ -301,7 +301,7 @@ export class UnitOfWork {
visited.add(entity);
const wrapped = entity.__helper!;

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

Expand Down Expand Up @@ -427,7 +427,7 @@ export class UnitOfWork {
if ([ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(prop.reference) && collection) {
collection
.getItems(false)
.filter(item => !requireFullyInitialized || item.__helper!.isInitialized())
.filter(item => !requireFullyInitialized || item.__helper!.__initialized)
.forEach(item => this.cascade(item, type, visited, options));
}
}
Expand Down Expand Up @@ -464,7 +464,7 @@ export class UnitOfWork {

const wrapped = entity.__helper!;

if (!wrapped.isInitialized()) {
if (!wrapped.__initialized) {
await wrapped.init();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/EntityHelper.mongo.test.ts
Expand Up @@ -76,7 +76,7 @@ describe('EntityHelperMongo', () => {

test('BaseEntity methods', async () => {
const god = new Author('God', 'hello@heaven.god');
expect(wrap(god, true).__populated).toBe(false);
expect(wrap(god, true).__populated).toBeUndefined();
god.populated();
expect(wrap(god, true).__populated).toBe(true);

Expand Down
7 changes: 2 additions & 5 deletions tests/EntityHelper.mysql.test.ts
@@ -1,4 +1,4 @@
import { MetadataDiscovery, MikroORM, wrap } from '@mikro-orm/core';
import { MikroORM, wrap } from '@mikro-orm/core';
import { MySqlDriver } from '@mikro-orm/mysql';
import { initORMMySql, wipeDatabaseMySql } from './bootstrap';
import { Author2, Book2, BookTag2, FooBar2, FooBaz2 } from './entities-sql';
Expand All @@ -7,10 +7,7 @@ describe('EntityHelperMySql', () => {

let orm: MikroORM<MySqlDriver>;

beforeAll(async () => {
orm = await initORMMySql();
await new MetadataDiscovery(orm.getMetadata(), orm.em.getDriver().getPlatform(), orm.config).discover();
});
beforeAll(async () => orm = await initORMMySql());
beforeEach(async () => wipeDatabaseMySql(orm.em));

test('assign() should update entity values [mysql]', async () => {
Expand Down
1 change: 1 addition & 0 deletions tests/EntityManager.mongo.test.ts
Expand Up @@ -504,6 +504,7 @@ describe('EntityManagerMongo', () => {
const driver = orm.em.getDriver();
expect(driver).toBeInstanceOf(MongoDriver);
expect(driver.getDependencies()).toEqual(['mongodb']);
expect(orm.config.getNamingStrategy().joinTableName('a', 'b', 'c')).toEqual('');
expect(await driver.find(BookTag.name, { foo: 'bar', books: 123 }, { orderBy: {} })).toEqual([]);
expect(await driver.findOne(BookTag.name, { foo: 'bar', books: 123 })).toBeNull();
expect(await driver.findOne(BookTag.name, { foo: 'bar', books: 123 }, { orderBy: {} })).toBeNull();
Expand Down

0 comments on commit 2228fcb

Please sign in to comment.