src: simplify NativeModule caching and remove redudant data by joyeecheung · Pull Request #25352 · nodejs/node

joyeecheung · 2019-01-05T15:43:19Z

Remove NativeModule._source - the compilation is now entirely
done in C++ and process.binding('natives') is implemented
directly in the binding loader so there is no need to store
additional source code strings.
Instead of using an object as NativeModule._cached and insert
into it after compilation of each native module, simply prebuild
a JS map filled with all the native modules and infer the
state of compilation through mod.loading/mod.loaded.
Rename NativeModule.nonInternalExists to
NativeModule.canBeRequiredByUsers and precompute that
property for all the native modules during bootstrap instead
of branching in every require call during runtime. This also fixes
the bug where worker_threads can be made available with
--expose-internals
Rename NativeModule.requireForDeps to
NativeModule.requireWithFallbackInDeps.
Add a test to make sure we do not accidentally leak any module
to the global namespace.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2019-01-05T15:43:21Z

@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2144/pipeline

mscdex · 2019-01-05T15:50:45Z

Typo in commit message: s/redudant/redundant/

joyeecheung · 2019-01-05T15:52:30Z

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

- Remove `NativeModule._source` - the compilation is now entirely done in C++ and `process.binding('natives')` is implemented directly in the binding loader so there is no need to store additional source code strings. - Instead of using an object as `NativeModule._cached` and insert into it after compilation of each native module, simply prebuild a JS map filled with all the native modules and infer the state of compilation through `mod.loading`/`mod.loaded`. - Rename `NativeModule.nonInternalExists` to `NativeModule.canBeRequiredByUsers` and precompute that property for all the native modules during bootstrap instead of branching in every require call during runtime. - Rename `NativeModule.requireForDeps` to `NativeModule.requireWithFallbackInDeps`. - Add a test to make sure we do not accidentally leak any module to the global namespace.

joyeecheung · 2019-01-09T15:46:20Z

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

joyeecheung · 2019-01-12T16:11:38Z

Landed in 92e95f1

- Remove `NativeModule._source` - the compilation is now entirely done in C++ and `process.binding('natives')` is implemented directly in the binding loader so there is no need to store additional source code strings. - Instead of using an object as `NativeModule._cached` and insert into it after compilation of each native module, simply prebuild a JS map filled with all the native modules and infer the state of compilation through `mod.loading`/`mod.loaded`. - Rename `NativeModule.nonInternalExists` to `NativeModule.canBeRequiredByUsers` and precompute that property for all the native modules during bootstrap instead of branching in every require call during runtime. This also fixes the bug where `worker_threads` can be made available with `--expose-internals`. - Rename `NativeModule.requireForDeps` to `NativeModule.requireWithFallbackInDeps`. - Add a test to make sure we do not accidentally leak any module to the global namespace. PR-URL: #25352 Reviewed-By: Anna Henningsen <anna@addaleax.net>

addaleax · 2019-01-15T23:10:51Z

@joyeecheung Can you look into backporting this to v11.x-staging?

BridgeAR · 2019-01-16T14:24:46Z

~~I pushed a backport directly onto v11.x-staging. There was just a minor conflict in lib/internal/bootstrap/loaders.js.~~

Seems like this requires a closer look.

BridgeAR · 2019-01-16T14:53:52Z

This should be backported along #25481.

addaleax · 2019-01-23T18:27:47Z

ping @joyeecheung

- Remove `NativeModule._source` - the compilation is now entirely done in C++ and `process.binding('natives')` is implemented directly in the binding loader so there is no need to store additional source code strings. - Instead of using an object as `NativeModule._cached` and insert into it after compilation of each native module, simply prebuild a JS map filled with all the native modules and infer the state of compilation through `mod.loading`/`mod.loaded`. - Rename `NativeModule.nonInternalExists` to `NativeModule.canBeRequiredByUsers` and precompute that property for all the native modules during bootstrap instead of branching in every require call during runtime. This also fixes the bug where `worker_threads` can be made available with `--expose-internals`. - Rename `NativeModule.requireForDeps` to `NativeModule.requireWithFallbackInDeps`. - Add a test to make sure we do not accidentally leak any module to the global namespace. PR-URL: nodejs#25352 Reviewed-By: Anna Henningsen <anna@addaleax.net>

- Remove `NativeModule._source` - the compilation is now entirely done in C++ and `process.binding('natives')` is implemented directly in the binding loader so there is no need to store additional source code strings. - Instead of using an object as `NativeModule._cached` and insert into it after compilation of each native module, simply prebuild a JS map filled with all the native modules and infer the state of compilation through `mod.loading`/`mod.loaded`. - Rename `NativeModule.nonInternalExists` to `NativeModule.canBeRequiredByUsers` and precompute that property for all the native modules during bootstrap instead of branching in every require call during runtime. This also fixes the bug where `worker_threads` can be made available with `--expose-internals`. - Rename `NativeModule.requireForDeps` to `NativeModule.requireWithFallbackInDeps`. - Add a test to make sure we do not accidentally leak any module to the global namespace. Backport-PR-URL: #25964 PR-URL: #25352 Reviewed-By: Anna Henningsen <anna@addaleax.net>

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jan 5, 2019

addaleax approved these changes Jan 5, 2019
View reviewed changes

Comment threadbenchmark/fixtures/require-cachable.js Outdated

addaleax mentioned this pull request Jan 6, 2019
modules: significantly improve require performance #25362
Closed
4 tasks

joyeecheung added 2 commits January 9, 2019 23:13

fixup! src: simplify NativeModule caching and remove redudant data
ffaf389

joyeecheung force-pushed the refactor-native branch from 9f0bbce to ffaf389Compare January 9, 2019 15:45

joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 12, 2019

joyeecheung closed this Jan 12, 2019

addaleax mentioned this pull request Jan 12, 2019
module: isolate paths expecting to only use non-internals #25449
Closed
4 tasks

addaleax added the backport-requested-v11.x label Jan 15, 2019

BridgeAR added backported-to-v11.x and removed backport-requested-v11.x labels Jan 16, 2019

BridgeAR mentioned this pull request Jan 16, 2019
src: remove unused variable #25481
Merged
2 tasks

targos mentioned this pull request Feb 6, 2019
Help wanted: Backporting to v11.x #25403
Closed

antsmartian mentioned this pull request Feb 6, 2019
[v11.x backport] src: simplify NativeModule caching and remove redundant data #25964
Closed
4 tasks

addaleax added backported-to-v11.x and removed backport-requested-v11.x labels Feb 9, 2019

targos mentioned this pull request Feb 14, 2019
v11.10.0 proposal #26098
Merged

Navigation Menu

Uh oh!

Uh oh!

src: simplify NativeModule caching and remove redudant data - #25352

src: simplify NativeModule caching and remove redudant data#25352
joyeecheung wants to merge 2 commits into
nodejs:masterfrom
joyeecheung:refactor-native

joyeecheung commented Jan 5, 2019•
edited
Loading

Uh oh!

nodejs-github-bot commented Jan 5, 2019

Uh oh!

mscdex commented Jan 5, 2019

Uh oh!

joyeecheung commented Jan 5, 2019

Uh oh!

Uh oh!

joyeecheung commented Jan 9, 2019

Uh oh!

joyeecheung commented Jan 12, 2019

Uh oh!

addaleax commented Jan 15, 2019

Uh oh!

BridgeAR commented Jan 16, 2019•
edited
Loading

Uh oh!

BridgeAR commented Jan 16, 2019

Uh oh!

addaleax commented Jan 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Uh oh!

Conversation

joyeecheung commented Jan 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

nodejs-github-bot commented Jan 5, 2019

Uh oh!

mscdex commented Jan 5, 2019

Uh oh!

joyeecheung commented Jan 5, 2019

Uh oh!

Uh oh!

joyeecheung commented Jan 9, 2019

Uh oh!

joyeecheung commented Jan 12, 2019

Uh oh!

addaleax commented Jan 15, 2019

Uh oh!

BridgeAR commented Jan 16, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BridgeAR commented Jan 16, 2019

Uh oh!

addaleax commented Jan 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joyeecheung commented Jan 5, 2019•
edited
Loading

BridgeAR commented Jan 16, 2019•
edited
Loading