fix: revert conversion of 'debug' package for 'node:util::debuglog' - Githubissues

koajs / router

Router middleware for Koa. Maintained by @forwardemail and @ladjs.

MIT License

849 stars 174 forks source link

fix: revert conversion of 'debug' package for 'node:util::debuglog' #173

Closed jeremy-daley-kr closed 10 months ago

jeremy-daley-kr commented 10 months ago

Description

A major performance regression was detected amounting to about 10x speed decrease. This occurred by the changing of how router is debugged.

Please see the following discussion: https://github.com/koajs/koa/discussions/1781

Checklist

[x] I have ensured my pull request is not behind the main or master branch of the original repository.
[x] I have rebased all commits where necessary so that reviewing this pull request can be done without having to merge it first.
[ ] I have written a commit message that passes commitlint linting.
[ ] I have ensured that my code changes pass linting tests.
[ ] I have ensured that my code changes pass unit tests.
[ ] I have described my pull request and the reasons for code changes along with context if necessary.

jeremy-daley-kr commented 10 months ago

@titanism Can you please look into what's failing here?

I don't believe it's from my PR, as I can see the same error in your commit from yesterday: https://github.com/koajs/router/commit/8d390a9ea4817e01f5a13a47d326a3d6887fe2b4

titanism commented 10 months ago

@jeremy-daley-kr can you file a GitHub issue in Node.js that asks why debuglog is so slow or something? Probably something event loop related?

titanism commented 10 months ago

Publishing v12.0.1 to npm now, almost done.

titanism commented 10 months ago

I'll most likely backport to v11 as patch too.

jeremy-daley-kr commented 10 months ago

@jeremy-daley-kr can you file a GitHub issue in Node.js that asks why debuglog is so slow or something? Probably something event loop related?

I'll see what I can do. Thanks for moving quickly with the merge. I'll keep my eye out for 12.0.1 👀

titanism commented 10 months ago

v12.0.1 released to npm and GitHub:

titanism commented 10 months ago

Deprecation notice published for all versions of koa-router and @koa/router (mirrored packages) of **IMPORTANT 10x+ PERFORMANCE UPGRADE**: Please upgrade to v12.0.1+ as we have fixed an issue with debuglog causing 10x slower router benchmark performance, see https://github.com/koajs/router/pull/173.

titanism commented 10 months ago

Manually backported and released as v11.0.1 for koa-router and @koa/router.

titanism commented 10 months ago

All done! 🎉 thanks again @jeremy-daley-kr - if you want some free credit for Forward Email just ping us at support@forwardemail.net once you sign up. 🖖

titanism commented 10 months ago

@MoFaro please read the deprecation and the issue. this is a warning to upgrade, not an error. it is a performance-related issue.

4nduril-ms commented 10 months ago

Sorry to bump that one. But is it intended that version 11.0.2 has the "latest" tag instead of 12.0.1?

titanism commented 10 months ago

@4nduril-ms amazing catch, I didn't specify --tag, fixing now, 🙏 - v12.0.2 releasing now

titanism commented 10 months ago

@4nduril-ms I actually didn't need to do another package version bump, there's npm dist-tag which was run successfully ✅

❯ npm dist-tag add @koa/router@12.0.1 latest
Authenticate your account at:
https://www.npmjs.com/auth/cli/xxx
Press ENTER to open in the browser...

+latest: @koa/router@12.0.1
❯ npm dist-tag add koa-router@12.0.1 latest
Authenticate your account at:
https://www.npmjs.com/auth/cli/xxx
Press ENTER to open in the browser...

+latest: koa-router@12.0.1

thanks again 🙏

4nduril-ms commented 10 months ago

@titanism Great! Thank you for fixing it that fast.

titanism commented 10 months ago

@4nduril-ms of course, also if you need anything email related, check out our project https://forwardemail.net 🔥