src: use dedicated routine to compile function for builtin CJS loader by joyeecheung · Pull Request #52016 · nodejs/node

joyeecheung · 2024-03-08T18:23:03Z

So that we can use it to handle code caching in a central place.

Needed by #47472, split out from #51977

Drive-by: use per-isolate persistent strings for the parameters and mark GetHostDefinedOptions() static since it's only used in one compilation unit

Refs: #47472

nodejs-github-bot · 2024-03-08T18:23:08Z

Review requested:

@nodejs/loaders
@nodejs/single-executable

joyeecheung · 2024-03-08T18:30:47Z

src/node_contextify.cc

+      params.data(),
+      0,       /* context extensions size */
+      nullptr, /* context extensions data */
+      // TODO(joyeecheung): allow optional eager compilation.


Interestingly, in my prototype for #47472, enabling eager compilation doesn't speed up the loading of the various CLI tools I found in the code base - actually, it's slower than the default (lazy compilation). But since #51672 showed that eager compilation of essential internals could at least speed up our core startup, I think this really depends on the usage pattern of the code being compiled, so I left a TODO for future investigation (also I think technically users can try eager compilation themselves using --no-lazy)

nodejs-github-bot · 2024-03-08T18:43:01Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57636/

So that we can use it to handle code caching in a central place. Drive-by: use per-isolate persistent strings for the parameters and mark GetHostDefinedOptions() since it's only used in one compilation unit

nodejs-github-bot · 2024-03-08T20:17:57Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57637/

GeoffreyBooth

This doesn’t need any tests?

joyeecheung · 2024-03-09T09:25:11Z

It's just a refactoring, if it doesn't break any tests, I think that's enough (and if it doesn't work there are bound to be test failure..)

nodejs-github-bot · 2024-03-10T00:25:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57650/

nodejs-github-bot · 2024-03-10T14:11:01Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57659/

nodejs-github-bot · 2024-03-11T13:02:51Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57680/

nodejs-github-bot · 2024-03-11T18:19:25Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57694/

nodejs-github-bot · 2024-03-11T19:18:54Z

Landed in bec9b5f

So that we can use it to handle code caching in a central place. Drive-by: use per-isolate persistent strings for the parameters and mark GetHostDefinedOptions() since it's only used in one compilation unit PR-URL: nodejs#52016 Refs: nodejs#47472 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

So that we can use it to handle code caching in a central place. Drive-by: use per-isolate persistent strings for the parameters and mark GetHostDefinedOptions() since it's only used in one compilation unit PR-URL: #52016 Refs: #47472 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

nodejs/node#52016

* chore: bump node in DEPS to v20.13.0 * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * chore: fixup patch indices * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: handle updated filenames - nodejs/node#51999 - nodejs/node#51927 * chore: bump node in DEPS to v20.13.1 * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: update patches --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

So that we can use it to handle code caching in a central place. Drive-by: use per-isolate persistent strings for the parameters and mark GetHostDefinedOptions() since it's only used in one compilation unit PR-URL: nodejs#52016 Refs: nodejs#47472 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

nodejs/node#52016

* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * chore: fixup patch indices * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: handle updated filenames * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup patch indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: remove stray log * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: fixup * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <zcbenz@gmail.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2024

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Mar 8, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2024

This comment was marked as outdated.

Sign in to view

joyeecheung commented Mar 8, 2024

View reviewed changes

joyeecheung force-pushed the cjs-compile branch from d0296c0 to 02b7b4d Compare March 8, 2024 18:34

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2024

src: use dedicated routine to compile function for builtin CJS loader

362df16

So that we can use it to handle code caching in a central place. Drive-by: use per-isolate persistent strings for the parameters and mark GetHostDefinedOptions() since it's only used in one compilation unit

joyeecheung force-pushed the cjs-compile branch from 02b7b4d to 362df16 Compare March 8, 2024 19:54

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 8, 2024

aduh95 approved these changes Mar 8, 2024

View reviewed changes

GeoffreyBooth approved these changes Mar 9, 2024

View reviewed changes

aymen94 approved these changes Mar 11, 2024

View reviewed changes

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 11, 2024

nodejs-github-bot merged commit bec9b5f into nodejs:main Mar 11, 2024

RafaelGSS mentioned this pull request Apr 15, 2024

2024-04-24, Version 22.0.0 (Current) #52505

Merged

marco-ippolito mentioned this pull request May 2, 2024

v20.13.0 proposal #52793

Merged

codebytere added a commit to electron/electron that referenced this pull request May 8, 2024

src: use dedicated routine to compile function for builtin CJS loader

c343fdd

nodejs/node#52016

codebytere added a commit to electron/electron that referenced this pull request May 31, 2024

src: use dedicated routine to compile function for builtin CJS loader

da84019

nodejs/node#52016

codebytere added a commit to electron/electron that referenced this pull request May 31, 2024

src: use dedicated routine to compile function for builtin CJS loader

65c01d3

nodejs/node#52016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

src: use dedicated routine to compile function for builtin CJS loader#52016

src: use dedicated routine to compile function for builtin CJS loader#52016
nodejs-github-bot merged 1 commit intonodejs:mainfrom
joyeecheung:cjs-compile

joyeecheung commented Mar 8, 2024 •

edited

Loading

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

This comment was marked as outdated.

joyeecheung Mar 8, 2024 •

edited

Loading

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

GeoffreyBooth left a comment

Uh oh!

joyeecheung commented Mar 9, 2024

Uh oh!

nodejs-github-bot commented Mar 10, 2024

Uh oh!

nodejs-github-bot commented Mar 10, 2024

Uh oh!

nodejs-github-bot commented Mar 11, 2024

Uh oh!

nodejs-github-bot commented Mar 11, 2024

Uh oh!

nodejs-github-bot commented Mar 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

joyeecheung commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

This comment was marked as outdated.

joyeecheung Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

nodejs-github-bot commented Mar 8, 2024

Uh oh!

GeoffreyBooth left a comment

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Mar 9, 2024

Uh oh!

nodejs-github-bot commented Mar 10, 2024

Uh oh!

nodejs-github-bot commented Mar 10, 2024

Uh oh!

nodejs-github-bot commented Mar 11, 2024

Uh oh!

nodejs-github-bot commented Mar 11, 2024

Uh oh!

nodejs-github-bot commented Mar 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

joyeecheung commented Mar 8, 2024 •

edited

Loading

joyeecheung Mar 8, 2024 •

edited

Loading