monkey-patching core objects is bad for performance

[wmertens]

varlock version

latest

Steps to reproduce

See https://github.com/dmno-dev/varlock/blob/main/packages/varlock/src/auto-load.ts#L5-L6 - Express changing the core objects actually gives a big performance hit, which is one of the reasons why fastify is faster, and I worry that these patches also do that.

What is expected?

Great DX, no runtime cost

What is actually happening?

Unknown, please benchmark to prove it's fine

System Info

Any additional comments?

No response

[philmillman]

Thanks for flagging @wmertens, this seems like more of a discussion than a bug per se. I'll move this over there and we can continue the conversation.

[theoephraim]

Thanks for bringing this up. We will do some benchmarking for sure!

There are also opt-out root decorators, which need to be added to the docs.
@redactLogs=false and @preventLeaks=false

[theoephraim]

Did some basic benchmarking on a few different setups.

All tests were using autocannon (pnpm exec autocannon -d 10 -c 50 http://localhost:3000) on my local machine. Obviously this is not very scientific and we should eventually have some automated tests we can run and report somewhere.

Each test has a server rendered page that is doing some kind of logging on the server, including a redacted secret.

Obviously this is a fairly trivial example, and the more logs you have and larger size of logs, as well as size of responses returned by the server are going to incur more of a hit, but this should at least give us a baseline idea of what is going on to just patch the methods.

• Interestingly, for qwik (using node adapter w/ fastify), there was basically no difference - the results are all about the same, and not obviously increasing w/ patching enabled.
• For astro, there is a small perf hit (<5%) to enable both patches. When I inject a leak detection middleware rather than patching globals (which you'd think should be the right way to do it) there is a very large perf hit compared to baseline of doing nothing. Maybe it would be more negligible in a non-trivial example... but still interesting!
• For react router 7, there is a 10-20% perf hit to enable both patches. Curious how this may change using different adapters.

The matrix here of frameworks, hosting platforms, javascript runtimes, etc gets huge pretty quickly.

Obviously it may change in prod versus on my laptop. But overall I'd say this is fairly promising!

[wmertens]

Well, I'm super happy that Qwik is not impacted in any case, but 10-20% is pretty severe IMHO. So at the very least it should be mentioned...

[theoephraim]

for sure! I'm going to dig into the RR7 adapters and see what's going on