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

Perf impact on module load seems to be depending on the number of hooks defined. #83

Open
pgayvallet opened this issue Mar 8, 2024 · 1 comment

Comments

@pgayvallet
Copy link
Contributor

During our investigations trying to optimize Kibana's startup time, we discovered that the perf impact of using require-in-the-middle was depending on the number of Hook that were defined. (More details in elastic/kibana#178285, but we're talking about ~20% performance gain in total module load time by using only one hook instead of 5)

The implementation of RITM seems to confirm it, given each instance of Hook is patching Module.prototype.require, therefor patching the already-patched require from the previous Hook.

I would expect the library to perform the patching only once, and handle all the registered hooks from this single patched method. It would avoid the interception logic to be executed each time for each hook, for each import.

@trentm
Copy link
Member

trentm commented Mar 11, 2024

Thanks for creating the issue!

I would expect the library to perform the patching only once, and handle all the registered hooks from this single patched method. It would avoid the interception logic to be executed each time for each hook, for each import.

Yes, definitely, that would be very nice to have.

It is for this performance reason that OpenTelemetry JS added a separate "RequireInTheMiddleSingleton" class to handle reducing the number of separate Hooks created: https://github.com/open-telemetry/opentelemetry-js/blob/1b4999f386e0240b7f65350e8360ccc2930b0fe6/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts#L45-L53

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants