Inversifyjs: Huge performance drop when having thousands calls to factory which resolves singletone dependencies

Created on 28 Nov 2016  路  6Comments  路  Source: inversify/InversifyJS

So, i have a project https://github.com/javascript-obfuscator/javascript-obfuscator/tree/dev and now i'm trying to move it on Inversify.

Inside some class i have code that traverses over AST tree (nodes) and for each node calls factory which returns array of node obfuscators for this node.

Without Inversify performance tests are returns following results:
1) 500 runs of program - ~8 sec total.
2) run with very large code size - ~1.8 sec.

With Inversify i got huge performance drop:
1) 500 runs of program - ~28 sec total.
2) run with very large code size - 5-6 sec.

With Inversify i created factory which resolves and returns instances of node obfuscators.
All instances are bind in singletone scope.

Now code examples.

Old code:

Factory call when traversing through ast tree
https://github.com/javascript-obfuscator/javascript-obfuscator/blob/dev/src/Obfuscator.ts#L116

Factory itself
https://github.com/javascript-obfuscator/javascript-obfuscator/blob/dev/src/node-transformers/AbstractNodeTransformersFactory.ts#L38

With Inversify

Factory call when traversing through ast tree
https://github.com/javascript-obfuscator/javascript-obfuscator/blob/inversify/src/Obfuscator.ts#L147

Factory itself
https://github.com/javascript-obfuscator/javascript-obfuscator/blob/inversify/src/container/InversifyContainerFacade.ts#L64

Binding setup for classes which factory is resolve
https://github.com/javascript-obfuscator/javascript-obfuscator/blob/inversify/src/container/modules/NodeObfuscatorsModule.ts#L15

So i'm doing something wrong?

Expected Behavior

Small performance drop.

Current Behavior

Huge performance drop.

Steps to Reproduce (for bugs)

You can clone this two branches
https://github.com/javascript-obfuscator/javascript-obfuscator/tree/dev
https://github.com/javascript-obfuscator/javascript-obfuscator/tree/inversify

then npm i && npm test

latest two asserts are performance asserts.

Most helpful comment

I think I might have an idea of what is going on... Your individual entities are singletons but your factory is not a singleton.

There is something that is not very clear in the docs: The factory is a singleton by default and there is no way to change that. However, the value returned by the factory may or may not be a factory.

The factory is just a function which is injected as a function always but when you invoke the factory the value returned may or may not be a singleton. The behavior is controlled by the way to implement your factory and it is not controlled by InversifyJS.

In your case is a bit more complicated because your factory uses named bindings but you can try the following (Please note that I have written the following code in notepad so it may contain errors).

this.container
    .bind<INodeTransformer[]>(ServiceIdentifiers['Factory<INodeTransformer[]>'])
    .toFactory<INodeTransformer[]>((context: interfaces.Context) => {

        let cache = new Map<string, INodeTransformer[]>(); // singleton!

        return (nodeTransformersMap: Map<string, string[]>) => {

            (nodeType: string) => {

                if (cache.has(nodeType)) {

                    // Try to access singleton from cache
                    return cache.get(nodeType);

                } else {

                    // Try to resolve

                    const nodeTransformers: string[] = nodeTransformersMap.get(nodeType) || [];
                    const instancesArray: INodeTransformer[] = [];

                    nodeTransformers.forEach((transformer: string) => {
                        instancesArray.push(
                            context.container.getNamed<INodeTransformer>(
                                'INodeTransformer', transformer
                            )
                        );
                    });

                    // Add to cache
                    cache.set(nodeType, instancesArray); // add to cache
                    return instancesArray;

                }

            };
        }
    });    

Please let me know if it improves the performance. Also please note that the performance should be good but is never going to be as fast as your original code because your original code didn't have to resolve a dependency tree at all. Everything was in memory as a singleton since the moment it was declared.

All 6 comments

Do you use version 2 or version 3 (current beta)?

Version 3 will have a big performance boost https://github.com/inversify/InversifyJS/issues/395#issuecomment-258036753

Version 3.0.0-beta.2

Hi @sanex3339 thanks for reporting this issue I will investigate and let you know my thoughts ASAP.

I think I might have an idea of what is going on... Your individual entities are singletons but your factory is not a singleton.

There is something that is not very clear in the docs: The factory is a singleton by default and there is no way to change that. However, the value returned by the factory may or may not be a factory.

The factory is just a function which is injected as a function always but when you invoke the factory the value returned may or may not be a singleton. The behavior is controlled by the way to implement your factory and it is not controlled by InversifyJS.

In your case is a bit more complicated because your factory uses named bindings but you can try the following (Please note that I have written the following code in notepad so it may contain errors).

this.container
    .bind<INodeTransformer[]>(ServiceIdentifiers['Factory<INodeTransformer[]>'])
    .toFactory<INodeTransformer[]>((context: interfaces.Context) => {

        let cache = new Map<string, INodeTransformer[]>(); // singleton!

        return (nodeTransformersMap: Map<string, string[]>) => {

            (nodeType: string) => {

                if (cache.has(nodeType)) {

                    // Try to access singleton from cache
                    return cache.get(nodeType);

                } else {

                    // Try to resolve

                    const nodeTransformers: string[] = nodeTransformersMap.get(nodeType) || [];
                    const instancesArray: INodeTransformer[] = [];

                    nodeTransformers.forEach((transformer: string) => {
                        instancesArray.push(
                            context.container.getNamed<INodeTransformer>(
                                'INodeTransformer', transformer
                            )
                        );
                    });

                    // Add to cache
                    cache.set(nodeType, instancesArray); // add to cache
                    return instancesArray;

                }

            };
        }
    });    

Please let me know if it improves the performance. Also please note that the performance should be good but is never going to be as fast as your original code because your original code didn't have to resolve a dependency tree at all. Everything was in memory as a singleton since the moment it was declared.

Yep, i done practically same thing simultaneously with your comment. Thank you for quick response and amazing Inversify!

Happy to help. I will close this issue.

Was this page helpful?
0 / 5 - 0 ratings