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

Migration of core modules to primordials #30697

Open
targos opened this issue Nov 28, 2019 · 64 comments
Open

Migration of core modules to primordials #30697

targos opened this issue Nov 28, 2019 · 64 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@targos
Copy link
Member

targos commented Nov 28, 2019

This is a tracking issue for the migration of core modules to use builtins from the primordials object.

At the moment, for performance-sensitive code, only static methods and global functions should be migrated. V8 8.0 might have the optimization that we need to migrate prototype methods.

@aduh95
Copy link
Contributor

aduh95 commented Nov 29, 2019

Is there somewhere I can find out about primordials and why/how it is better than using global builtins? Maybe a recording of the meeting when this decision was taken? Is the primordials object documented somewhere?

@amitmtrn
Copy link

@aduh95
from my understanding it made in order to avoid user mutation in core libraries

// This file subclasses and stores the JS builtins that come from the VM
// so that Node.js's builtin modules do not need to later look these up from
// the global proxy, which can be mutated by users.

targos added a commit that referenced this issue Nov 30, 2019
Refs: #30697

PR-URL: #30698
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
targos added a commit to targos/node that referenced this issue Nov 30, 2019
targos added a commit that referenced this issue Dec 1, 2019
Refs: #30697

PR-URL: #30698
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
targos added a commit that referenced this issue Dec 13, 2019
PR-URL: #30882
Refs: #30697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
MylesBorins pushed a commit that referenced this issue Dec 13, 2019
PR-URL: #30882
Refs: #30697
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
targos added a commit that referenced this issue Dec 16, 2019
PR-URL: #30936
Refs: #30697
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this issue Dec 17, 2019
PR-URL: #30936
Refs: #30697
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@ljharb
Copy link
Member

ljharb commented Oct 1, 2021

I would love to, but unfortunately that time conflicts with childcare duties. I could potentially call in towards the later part of that hour - after 11:40AM EDT or so - if the meeting goes that long. Is that an option?

@gireeshpunathil
Copy link
Member

thanks @ljharb . Ideally the meeting runs for an hour, so it is technically possible to adjust the agenda to meet this need. Also I don't think the pitch needs all the three proponents to be available at the same time. I will work with the chair to accommodate this. thanks!

@gireeshpunathil
Copy link
Member

@ljharb (@bmeck , @BridgeAR) - correction to my previous comment, the TSC sitting is happening on 6th, not on 7th. Does it make things better or worse for you?

@ljharb
Copy link
Member

ljharb commented Oct 5, 2021

Typically i'd have that whole hour free, but this particular Wednesday I also do childcare, so I'll be able to call in the last 20-15 minutes of that hour (any time between ~11:45AM ET - 12:30PM ET)

@bmeck
Copy link
Member

bmeck commented Oct 5, 2021

This should be fine for me

@BridgeAR
Copy link
Member

BridgeAR commented Oct 5, 2021

I can not guarantee to be able to participate tomorrow due to some work obligations.

@gireeshpunathil
Copy link
Member

is there a link for non TSC members?

pls standby, I will let you know in few mins.

@gireeshpunathil
Copy link
Member

just sent you the details over mail, pls check

@js-choi
Copy link

js-choi commented Oct 6, 2021

@gireeshpunathil: I’m the champion of the new bind-this operator proposal that is partially motivated by Node’s primordials use case, and which I will present to the next TC39 plenary. I’d like to listen in on the TSC meeting too, if that would be appropriate.

@aduh95
Copy link
Contributor

aduh95 commented Oct 6, 2021

It's being discussed right now, you can follow along on the public stream: https://www.youtube.com/c/nodejs+foundation/live

@Trott
Copy link
Member

Trott commented Dec 2, 2021

Per discussion at the TSC meeting today, we need more @nodejs/tsc folks to get involved in identifying all viable options (and eliminating options that are not viable).

@aduh95
Copy link
Contributor

aduh95 commented Dec 2, 2021

In a previous TSC meeting there was a question regarding if it was possible to use primordials only in internal modules related to ESM implementation. Here's the list of all the dependencies of the internal/modules/esm/* modules:

  • async_hooks
  • buffer
  • crypto
  • events
  • fs
  • internal/abort_controller
  • internal/assert
  • internal/async_hooks
  • internal/blob
  • internal/bootstrap
  • internal/buffer
  • internal/cli_table
  • internal/console
  • internal/constants
  • internal/crypto
  • internal/encoding
  • internal/errors
  • internal/event_target
  • internal/fixed_queue
  • internal/fs
  • internal/heap_utils
  • internal/idna
  • internal/linkedlist
  • internal/modules
  • internal/options
  • internal/perf
  • internal/policy
  • internal/priority_queue
  • internal/process
  • internal/promise_hooks
  • internal/querystring
  • internal/readline
  • internal/source_map
  • internal/stream_base_commons
  • internal/streams
  • internal/timers
  • internal/tty
  • internal/url
  • internal/util
  • internal/validators
  • internal/vm
  • internal/webstreams
  • internal/worker
  • os
  • path
  • querystring
  • stream
  • string_decoder
  • timers
  • url
  • util
  • v8
  • vm

So it's a long list, you can also check the visual dependecy graph if that helps.

@mcollina
Copy link
Member

mcollina commented Dec 3, 2021

In fairness, I think the API footprint is significantly lower and some of those modules are not strictly used but pulled for other APIs.

@mhdawson
Copy link
Member

Removed from TSC agenda as we tagged the issue in the TSC repo for the vote instead. That issue refers to this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.