Improve performance of accessing globals in scripts running in vm modules

[SimenB]

Is your feature request related to a problem? Please describe.

Accessing globals from within a vm.Script is guarded by interceptors which makes accessing them slow, (from my understanding, @fhinkel has an excellent comment explaining some of this here: jestjs/jest#5163 (comment)). The suggested workaround is to evaluate and cache the global you want access to (Math in my example case below) and inject that into the vm.Script. This is an acceptable workaround for vm.Script and vm.compileFunction but it cannot be done when using ESM vm.SourceTextModule as there is no wrapping function call.

I've put together this script you can run to see the numbers:

// test.js
const {
  compileFunction,
  createContext,
  runInContext,
  SourceTextModule
} = require("vm");
const context = createContext({ console });

const sourceCode = `
  const runTimes = 10000000;
  let sum = 0;
  console.time();
  for (let i = 0; i < runTimes; i++) {
    sum += Math.abs(0);
  }
  console.timeEnd();
`;

(async () => {
  const compiledFunction = compileFunction(sourceCode, [], {
    parsingContext: context
  });

  const vmModule = new SourceTextModule(sourceCode, { context });

  await vmModule.link(() => {
    throw new Error("should never be called");
  });

  // These are both super slow (1.6 seconds on my machine)
  compiledFunction();
  await vmModule.evaluate();

  // however, this is quick (less than 10 milliseconds on my machine)
  const compiledWithMathFunction = compileFunction(sourceCode, ["Math"], {
    parsingContext: context
  });
  compiledWithMathFunction(runInContext("Math", context));
})().catch(error => {
  console.error(error);
  process.exitCode = 1;
});

Running this gives the following results on my machine:

$ node --experimental-vm-modules test.js
(node:19958) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
default: 1.698s
default: 1.620s
default: 7.306ms

So ~1600ms if not using the workaround, which reduces it to ~7ms.

Describe the solution you'd like

I'd like for accessing globals to be as fast, or as close to as possible, as fast as accessing globals outside of vm. Would it be possible for Node's vm implementation to cache the global property lookups, so that the price is only paid once instead of on every single access?

Describe alternatives you've considered

As mentioned, caching and injecting the global manually works when there is a function wrapper, but this doesn't work with ESM. I've tried assigning the Math global to the context, and while that halves the time spent (~800ms), it's still 2 orders of magnitude slower than injecting a reference.

[devsnek]

There is nothing node can do to help performance here. The problem is that V8's global proxy design forces the embedder to indirect through c++ for every lookup, which adds a lot of overhead.

[SimenB]

And there's no way to cache/circumvent that lookup i node? If not, do you think there's any chance of v8 making changes so we can work around it?

Or are there any workarounds similar to the current workaround we can for SourceTextModule (or whatever the ESM vm API ends up being)?

[devsnek]

@SimenB You could do const context = createContext(); context.console = console instead of const context = createContext({ console }).

In the long term V8 might need to refactor this system anyway because of the upcoming realms API, so maybe one day createContext({}) will be fast too.

[SimenB]

@devsnek I mentioned that in the OP - it takes the time from 1600ms to 800ms (so halves the time), but it's still 2 orders of magnitude slower than injecting it (which is less than 10ms).

I see no difference between createContext({ someGlobal }) and createContext().someGlobal = someGlobal - both are twice as fast as not assigning the global at all, but still suuuuper slow compared to running outside of vm all together.

[devsnek]

@SimenB it seems that at some point we started creating the internal proxy unconditionally, which is why you're not seeing a speedup with what i suggested.

[devsnek]

Using the vm.Context constructor from #30709 also fixes this. Hopefully I can get that merged soon.

[SimenB]

Ah, wonderful!

[SimenB]

I just compiled your branch and can confirm it does indeed fix the issue 😀

🤞 you're able to land it! Might be a bit premature, but do you think it'll be backported to v10 and v12? Since it's a different API graceful degradation should be fine, but the free performance boost for all release line would be wonderful (and Jest could remove the option we added to allow people to work around this)

[juanarbol]

Fixed on 4725ac6 :)

[juanarbol]

Sorry, @SimenB is this issue solved?

[SimenB]

I don't think so, #30709 was closed unmerged?

[SimenB]

Please land it, it would make me very happy 😀

[TimothyGu]

Also note that this issue is known as far back as 2016: nodejs/benchmarking#75 (comment).

[thernstig]

Considering it was 1,5 years ago #30709 was closed in favor of the realms API, has the realms API landed or should @devsnek's PR still be worked on?