Skip to content

Commit 77fb3ba

Browse files
fix(core) improve topological sorting performance
1 parent 12368ff commit 77fb3ba

6 files changed

Lines changed: 37 additions & 64 deletions

File tree

integration/hello-world/e2e/middleware-execute-order.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('Middleware (execution order)', () => {
4747
await app.init();
4848
});
4949

50-
it(`should execute middleware of modules that are closer to the root module`, () => {
50+
it(`should execute middleware in topological order`, () => {
5151
return request(app.getHttpServer())
5252
.get('/hello')
5353
.expect(200, RETURN_VALUE_B);

packages/core/injector/module.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,8 @@ export class Module {
120120
return this._distance;
121121
}
122122

123-
public updateDistance(distance: number, stack: Module[]) {
124-
this._distance = distance;
125-
Array.from(this._imports)
126-
.filter(module => module && !stack.includes(this))
127-
.forEach(module =>
128-
module.updateDistance(distance + 1, stack.concat(this)),
129-
);
123+
set distance(value: number) {
124+
this._distance = value;
130125
}
131126

132127
public addCoreProviders(container: NestContainer) {
@@ -417,9 +412,6 @@ export class Module {
417412

418413
public addRelatedModule(module: Module) {
419414
this._imports.add(module);
420-
if (this._distance + 1 > module._distance) {
421-
module.updateDistance(this._distance + 1, [this]);
422-
}
423415
}
424416

425417
public replace(toReplace: string | symbol | Type<any>, options: any) {

packages/core/middleware/middleware-module.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,15 @@ export class MiddlewareModule {
6868
modules: Map<string, Module>,
6969
) {
7070
const moduleEntries = [...modules.entries()];
71-
await Promise.all(
72-
moduleEntries.map(async ([name, module]) => {
73-
const instance = module.instance;
74-
75-
await this.loadConfiguration(middlewareContainer, instance, name);
76-
await this.resolver.resolveInstances(module, name);
77-
}),
78-
);
71+
const loadMiddlewareConfiguration = async ([name, module]: [
72+
string,
73+
Module,
74+
]) => {
75+
const instance = module.instance;
76+
await this.loadConfiguration(middlewareContainer, instance, name);
77+
await this.resolver.resolveInstances(module, name);
78+
};
79+
await Promise.all(moduleEntries.map(loadMiddlewareConfiguration));
7980
}
8081

8182
public async loadConfiguration(

packages/core/middleware/resolver.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ export class MiddlewareResolver {
1212
const middleware = this.middlewareContainer.getMiddlewareCollection(
1313
moduleName,
1414
);
15-
await Promise.all(
16-
[...middleware.values()].map(async wrapper =>
17-
this.resolveMiddlewareInstance(wrapper, middleware, module),
18-
),
19-
);
15+
const resolveInstance = async (wrapper: InstanceWrapper) =>
16+
this.resolveMiddlewareInstance(wrapper, middleware, module);
17+
await Promise.all([...middleware.values()].map(resolveInstance));
2018
}
2119

2220
private async resolveMiddlewareInstance(

packages/core/scanner.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
import { ApplicationConfig } from './application-config';
3737
import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR, APP_PIPE } from './constants';
3838
import { CircularDependencyException } from './errors/exceptions/circular-dependency.exception';
39+
import { ModulesContainer } from './injector';
3940
import { NestContainer } from './injector/container';
4041
import { InstanceWrapper } from './injector/instance-wrapper';
4142
import { Module } from './injector/module';
@@ -116,6 +117,7 @@ export class DependenciesScanner {
116117
this.reflectControllers(metatype, token);
117118
this.reflectExports(metatype, token);
118119
}
120+
this.calculateModulesDistance(modules);
119121
}
120122

121123
public async reflectImports(
@@ -251,6 +253,26 @@ export class DependenciesScanner {
251253
return undefined;
252254
}
253255

256+
public async calculateModulesDistance(modules: ModulesContainer) {
257+
const modulesStack = [];
258+
const modulesGenerator = modules.values();
259+
const rootModule = modulesGenerator.next().value;
260+
261+
const calculateDistance = (moduleRef: Module, distance = 1) => {
262+
if (modulesStack.includes(moduleRef)) {
263+
return;
264+
}
265+
modulesStack.push(moduleRef);
266+
267+
const moduleImports = rootModule.relatedModules;
268+
moduleImports.forEach(module => {
269+
module.distance = distance;
270+
calculateDistance(module, distance + 1);
271+
});
272+
};
273+
calculateDistance(rootModule);
274+
}
275+
254276
public async insertImport(related: any, token: string, context: string) {
255277
if (isUndefined(related)) {
256278
throw new CircularDependencyException(context);

packages/core/test/injector/module.spec.ts

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -481,44 +481,4 @@ describe('Module', () => {
481481
});
482482
});
483483
});
484-
485-
describe('getter "distance"', () => {
486-
@ModuleDecorator({})
487-
class ModuleA {}
488-
489-
@ModuleDecorator({})
490-
class ModuleB {}
491-
492-
@ModuleDecorator({})
493-
class ModuleC {}
494-
495-
let moduleA, moduleB, moduleC;
496-
beforeEach(() => {
497-
moduleA = new Module(ModuleA as any, [], container);
498-
moduleB = new Module(ModuleB as any, [], container);
499-
moduleC = new Module(ModuleC as any, [], container);
500-
});
501-
502-
it('should calculate "distance" properly', () => {
503-
moduleA.addRelatedModule(moduleB);
504-
moduleC.addRelatedModule(moduleB);
505-
moduleA.addRelatedModule(moduleC);
506-
507-
expect(moduleA.distance).to.be.equal(0);
508-
expect(moduleB.distance).to.be.equal(2);
509-
expect(moduleC.distance).to.be.equal(1);
510-
});
511-
512-
it('should not throw an exception when circular dependency occurs', () => {
513-
expect(() => {
514-
moduleA.addRelatedModule(moduleB);
515-
moduleB.addRelatedModule(moduleA);
516-
}).to.not.throw;
517-
expect(() => {
518-
moduleA.addRelatedModule(moduleB);
519-
moduleB.addRelatedModule(moduleC);
520-
moduleC.addRelatedModule(moduleA);
521-
}).to.not.throw;
522-
});
523-
});
524484
});

0 commit comments

Comments
 (0)