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

vm: perform lazy-loading at top level #53587

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RedYetiDev
Copy link
Member

As it currently stands, the lazy loading for lib/internal/vm/module.js's require of internal/util/inspect is performed in a function. This means that a re-call of the function will be a re-call of the require of the module.

This PR moves the lazy-loading to only occur once.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vm Issues and PRs related to the vm subsystem. labels Jun 25, 2024
@atlowChemi
Copy link
Member

@RedYetiDev How big a difference is it considering the require cache?

@RedYetiDev
Copy link
Member Author

top_level.js

const vm = require('vm');

for (let i = 0; i < 1_000_000; i++) {
    vm;
    1+1;
}

vs

in_func.js

for (let i = 0; i < 1_000_000; i++) {
    const vm = require('vm');
    1+1;
}

I got:

Benchmark 1: node in_func.js
  Time (mean ± σ):     218.0 ms ±  10.2 ms    [User: 219.2 ms, System: 12.4 ms]
  Range (min … max):   201.5 ms … 228.3 ms    14 runs
 
Benchmark 2: node top_level.js
  Time (mean ± σ):      17.9 ms ±   0.5 ms    [User: 12.1 ms, System: 6.9 ms]
  Range (min … max):    17.0 ms …  21.1 ms    162 runs
 
Summary
  node top_level.js ran
   12.20 ± 0.66 times faster than node in_func.js

Note this benchmark is in no way accurate, it's just a basic demo, and it was really just put together in 10 seconds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants