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

fix: made type-metadata.storage.ts much faster #2212

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
@@ -0,0 +1,15 @@
export class ArrayCollection<T> extends Array<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending built-in collections is typically considered a bad practice. Any reason we cant use Array directly?

Copy link
Contributor Author

@roypeled roypeled Jun 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, for every collection that we are saving per metadata class, we also need to save a global collection of all the classes collections in a flat array.
I thought the easiest way to do that is to hook on arrays push and I shift methods, but it maybe bette to have this class create an internal array and expose the required methods

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed another fix, now I'm wrapping the array instead of extending it. Good comment!

constructor(private globalArray: Array<T>) {
super();
}

push(...items): number {
this.globalArray.push(...items);
return super.push(...items);
}

unshift(...items): number {
this.globalArray.unshift(...items);
return super.unshift(...items);
}
}
@@ -0,0 +1,18 @@
import { MetadataListByNameCollection } from './metadata.list.by.name.collection';
import { PropertyDirectiveMetadata } from '../metadata';

export class FieldDirectiveCollection extends MetadataListByNameCollection<PropertyDirectiveMetadata> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename file to field-directive.collection.ts

sdls = new Set<string>();
fieldNames = new Set<string>();

add(value: PropertyDirectiveMetadata) {
if (this.sdls.has(value.sdl) && this.fieldNames.has(value.fieldName))
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (this.sdls.has(value.sdl) && this.fieldNames.has(value.fieldName))
return;
if (this.sdls.has(value.sdl) && this.fieldNames.has(value.fieldName)) {
return;
}

style: for the sake of consistency


super.add(value, value.fieldName);

this.sdls.add(value.sdl);
this.fieldNames.add(value.fieldName);
this.globalArray && this.globalArray.push(value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.globalArray && this.globalArray.push(value);
this.globalArray?.push(value);

}
}
7 changes: 7 additions & 0 deletions packages/graphql/lib/schema-builder/collections/index.ts
@@ -0,0 +1,7 @@
export * from './metadata.storage.collection.list';
export * from './field.directive.collection';
export * from './array.collection';
export * from './metada.collection.model.interface';
export * from './metadata.by.name.collection';
export * from './metadata.list.by.name.collection';
export * from './metadata.storage.collection';
@@ -0,0 +1,14 @@
import { ClassMetadata, ResolverClassMetadata } from '../metadata';
import { ObjectTypeMetadata } from '../metadata/object-type.metadata';

export interface MetadataCollectionModelInterface {
argumentType: ClassMetadata[];
interface: ClassMetadata[];
inputType: ClassMetadata[];
objectType: ObjectTypeMetadata[];
resolver: ResolverClassMetadata[];
classDirectives: [];
classExtensions: [];
fieldDirectives: [];
fieldExtensions: [];
}
@@ -0,0 +1,26 @@
export class MetadataByNameCollection<T> {
protected map = new Map<string, T>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected all: (T extends any[] ? T[number] : T)[] = [];

getAll() {
return this.all;
}

getByName(name: string) {
return this.map.get(name);
}

add(value: T extends any[] ? T[number] : T, name: string) {
if (this.map.has(name)) return;

this.map.set(name, value);
this.all.push(value);
}

unshift(value: T extends any[] ? T[number] : T, name: string) {
if (this.map.has(name)) return;

this.map.set(name, value);
this.all.unshift(value);
}
}
@@ -0,0 +1,37 @@
import { MetadataByNameCollection } from './metadata.by.name.collection';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename file to metadata-list-by-name.collection.ts


export class MetadataListByNameCollection<T> extends MetadataByNameCollection<
T[]
> {
constructor(protected globalArray: Array<T> = null) {
super();
}

getByName(name: string): T[] {
return super.getByName(name) || [];
}

add(value: T, name: string) {
let arrayResult = super.getByName(name);
if (!arrayResult) {
arrayResult = [];
this.map.set(name, arrayResult);
}

arrayResult.push(value);
this.all.push(value);
this.globalArray && this.globalArray.push(value);
}

unshift(value: T, name: string) {
let arrayResult = super.getByName(name);
if (!arrayResult) {
arrayResult = [];
this.map.set(name, arrayResult);
}

arrayResult.unshift(value);
this.all.push(value);
this.globalArray && this.globalArray.unshift(value);
}
}
@@ -0,0 +1,44 @@
import { MetadataStorageCollection } from './metadata.storage.collection';
import { MetadataCollectionModelInterface } from './metada.collection.model.interface';

export class MetadataStorageCollectionList {
private storageMap = new Map<Function, MetadataStorageCollection>();
private storageList = new Array<MetadataStorageCollection>();

public all: MetadataCollectionModelInterface = {
argumentType: [],
interface: [],
inputType: [],
objectType: [],
resolver: [],
classDirectives: [],
classExtensions: [],
fieldDirectives: [],
fieldExtensions: [],
};

get(target: Function) {
let metadata = this.storageMap.get(target);

if (!metadata) {
metadata = new MetadataStorageCollection(this.all);
this.storageMap.set(target, metadata);
this.storageList.push(metadata);
}

return metadata;
}

compile() {
this.reversePredicate((t) => t.classDirectives);
this.reversePredicate((t) => t.classExtensions);
this.reversePredicate((t) => t.fieldDirectives.getAll());
this.reversePredicate((t) => t.fieldExtensions.getAll());
}

private reversePredicate<V>(
predicate: (t: MetadataStorageCollection) => Array<V>,
) {
this.storageList.forEach((t) => predicate(t).reverse());
}
}
@@ -0,0 +1,79 @@
import { MetadataByNameCollection } from './metadata.by.name.collection';
import {
ClassDirectiveMetadata,
ClassExtensionsMetadata,
ClassMetadata,
MethodArgsMetadata,
PropertyExtensionsMetadata,
PropertyMetadata,
ResolverClassMetadata,
} from '../metadata';
import { MetadataListByNameCollection } from './metadata.list.by.name.collection';
import { FieldDirectiveCollection } from './field.directive.collection';
import { ObjectTypeMetadata } from '../metadata/object-type.metadata';
import { ArrayCollection } from './array.collection';
import { MetadataCollectionModelInterface } from './metada.collection.model.interface';

export class MetadataStorageCollection {
constructor(private all: MetadataCollectionModelInterface) {}

fields = new MetadataByNameCollection<PropertyMetadata>();
params = new MetadataListByNameCollection<MethodArgsMetadata>();
fieldDirectives = new FieldDirectiveCollection(this.all.fieldDirectives);
fieldExtensions =
new MetadataListByNameCollection<PropertyExtensionsMetadata>(
this.all.fieldExtensions,
);
classDirectives = new ArrayCollection<ClassDirectiveMetadata>(
this.all.classDirectives,
);
classExtensions = new ArrayCollection<ClassExtensionsMetadata>(
this.all.classExtensions,
);

set argumentType(val: ClassMetadata) {
this._argumentType = val;
this.all.argumentType.push(val);
}
get argumentType() {
return this._argumentType;
}

set interface(val: ClassMetadata) {
this._interface = val;
this.all.interface.push(val);
}
get interface() {
return this._interface;
}

set inputType(val: ClassMetadata) {
this._inputType = val;
this.all.inputType.push(val);
}
get inputType() {
return this._inputType;
}

set objectType(val: ObjectTypeMetadata) {
this._objectType = val;
this.all.objectType.push(val);
}
get objectType() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing blank lines between getters and setters

return this._objectType;
}

set resolver(val: ResolverClassMetadata) {
this._resolver = val;
this.all.resolver.push(val);
}
get resolver() {
return this._resolver;
}

_argumentType: ClassMetadata;
_interface: ClassMetadata;
_inputType: ClassMetadata;
_objectType: ObjectTypeMetadata;
_resolver: ResolverClassMetadata;
}