-
Notifications
You must be signed in to change notification settings - Fork 134
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
Node.js Technical Steering Committee (TSC) Meeting 2022-12-14 #1319
Comments
Sorry this is late to be opened. |
There are only two issues on the agenda and we discussed them last week. |
I'll be there long enough to at least decide we don't need to have the meeting. 😀 |
I'll be there but slightly late. |
I'll be late or cannot attend, as an update for startup performance:
|
@joyeecheung Can we write down somewhere, perhaps in https://github.com/nodejs/node/tree/main/doc/contributing, how to avoid these regressions in the future? Like a guide to how to make modules snapshottable, how to add them to the snapshot, how and when to lazy-load things, how and when to avoid circular dependencies, etc. cc @nodejs/performance |
Did I miss the meeting? |
The meeting just ended |
See the comments in https://github.com/nodejs/node/blob/main/lib/internal/bootstrap/node.js - I suspect this place is much more noticeable than any doc we have in the repo (I read an old version of it before I started to do any work in startup, and it was also mention in blogs/books). The file itself is one of the oldest in the code base (if we disregard the exact directory where it is).
That probably should go to |
PR for minutes - #1320 |
We have a test that indicates when new modules are added to the startup. I originally wrote it when starting to lazy load modules to improve the startup time before. Shall we have a stronger indicator to prevent this in the future? |
I think the issue is that we have the list but we still didn't care much when it was extended. Also the test does not show how the modules were actually loaded (e.g. from snapshot, or being compiled at runtime, the latter is much more expensive even with the help of code cache, the former is cheaper, but not free). |
I think another cause was that since we stopped tracking the performance of main/releases in general, we stopped noticing about regressions like this (to catch nodejs/node#45716 in time we needed to run the benchmarks after every V8 update, to catch nodejs/node#45659 and nodejs/node#45849 in time we needed constant monitoring of the performance because it happened gradually, one module would not significantly slow down the startup but when we gradually doubled the number of internal modules, the impact would be significant). |
I’ve never heard about this file until now, and I’ve been working in this codebase for something like five years now. We could at least link to this file from the contributor guide. |
You've been missing all the fun ;) The comments even start with
Do you want to write a guide on hacking |
I don’t have the time or knowledge to write a guide on hacking I’m happy to open the PR to the contributor guide to just add a link to this file, unless someone else wants to beat me to it. I’m not sure when I’ll be able to get to it. |
Time
UTC Wed 14-Dec-2022 16:00 (04:00 PM):
Or in your local time:
Links
Agenda
Extracted from tsc-agenda labelled issues and pull requests from the nodejs org prior to the meeting.
nodejs/node
Invited
Observers/Guests
Notes
The agenda comes from issues labelled with
tsc-agenda
across all of the repositories in the nodejs org. Please label any additional issues that should be on the agenda before the meeting starts.Joining the meeting
Zoom link: https://zoom.us/j/611357642
Regular password
Public participation
We stream our conference call straight to YouTube so anyone can listen to it live, it should start playing at https://www.youtube.com/c/nodejs+foundation/live when we turn it on. There's usually a short cat-herding time at the start of the meeting and then occasionally we have some quick private business to attend to before we can start recording & streaming. So be patient and it should show up.
Invitees
Please use the following emoji reactions in this post to indicate your
availability.
The text was updated successfully, but these errors were encountered: