Repository: vuejs/vue
Pull request: use vm.Script to avoid reparse

DISCLAIMER: I don't have enough knowledge in nodejs nor V8. I read most cpp/internal js  is by guessing and cramming on cppreference.com :/  I welcome any correction!

This pr tries to alleviate the problem in https://github.com/vuejs/vue/issues/4872. When users include third party library into server bundle, the parsing overhead can be smaller.

If user provided global variable is compared with another global variable in library code, they will never be equal because they are executed in different context.

To fix this generally, we have to run library code and user code in the same context. This means we need to parse a lot of code in one request. 

We can skip parsing by creating a `Script` object, and `runInNewContext` is [effectively a shortcut](https://github.com/nodejs/node/blob/5de3cf099cd01c84d1809dab90c041b76aa58d8e/lib/vm.js#L62-L65). Thus it is safe to reuse the `Script` object.

Reusing `Script` should save several C++ call and reduce garbage collection pressure since the underlying C++ object has a [`Persistent`](https://github.com/nodejs/node/blob/5de3cf099cd01c84d1809dab90c041b76aa58d8e/src/node_contextify.cc#L461) which will not be collected until [out of memory](https://github.com/nodejs/node-v0.x-archive/issues/5080) (it is cargo cult to me, though).

However, I **cannot see a performance improvement** in a simple benchmark,  regardless of whether bundling library code into server entry. (I use vue-hackernews as example, tested under `ab -n 1000 -c 100`).
It seems v8 will [cache compiled code](https://github.com/v8/v8/blob/30224360c10ddd065f46eb81fffa69b88e5378d2/src/compiler.cc#L1571-L1573). So parsing overhead is relatively ignorable in real world.

But reusing object is still a good practice, so I submitted this pull request for symbolic value.

Changed files:
filename: src/server/create-bundle-renderer.js
patch: @@ -41,13 +41,14 @@ export function createBundleRendererCreator (createRenderer: () => Renderer) {
     } else {
       throw new Error(INVALID_MSG)
     }
+    const evaluate = runInVm(entry, files)
     return {
       renderToString: (context?: Object, cb: (err: ?Error, res: ?string) => void) => {
         if (typeof context === 'function') {
           cb = context
           context = {}
         }
-        runInVm(entry, files, context).catch(err => {
+        evaluate(context).catch(err => {
           rewriteErrorTrace(err, maps)
           cb(err)
         }).then(app => {
@@ -61,7 +62,7 @@ export function createBundleRendererCreator (createRenderer: () => Renderer) {
       },
       renderToStream: (context?: Object) => {
         const res = new PassThrough()
-        runInVm(entry, files, context).catch(err => {
+        evaluate(context).catch(err => {
           rewriteErrorTrace(err, maps)
           // avoid emitting synchronously before user can
           // attach error listener

filename: src/server/run-in-vm.js
patch: @@ -19,39 +19,55 @@ function createContext (context) {
   return sandbox
 }
 
-function evaluateModule (filename, files, context, evaluatedModules) {
-  if (evaluatedModules[filename]) {
-    return evaluatedModules[filename]
-  }
+function compileModule (files) {
+  const compiledScripts = {}
 
-  const code = files[filename]
-  const wrapper = NativeModule.wrap(code)
-  const compiledWrapper = vm.runInNewContext(wrapper, context, {
-    filename,
-    displayErrors: true
-  })
-  const m = { exports: {}}
-  const r = file => {
-    file = path.join('.', file)
-    if (files[file]) {
-      return evaluateModule(file, files, context, evaluatedModules)
-    } else {
-      return require(file)
+  function getCompiledScript (filename) {
+    if (compiledScripts[filename]) {
+      return compiledScripts[filename]
     }
+    const code = files[filename]
+    const wrapper = NativeModule.wrap(code)
+    const script = new vm.Script(wrapper, {
+      filename,
+      displayErrors: true
+    })
+    compiledScripts[filename] = script
+    return script
   }
-  compiledWrapper.call(m.exports, m.exports, r, m)
 
-  const res = Object.prototype.hasOwnProperty.call(m.exports, 'default')
-    ? m.exports.default
-    : m.exports
-  evaluatedModules[filename] = res
-  return res
+  function evaluateModule (filename, context, evaluatedModules) {
+    if (evaluatedModules[filename]) {
+      return evaluatedModules[filename]
+    }
+
+    const script = getCompiledScript(filename)
+    const compiledWrapper = script.runInNewContext(context)
+    const m = { exports: {}}
+    const r = file => {
+      file = path.join('.', file)
+      if (files[file]) {
+        return evaluateModule(file, context, evaluatedModules)
+      } else {
+        return require(file)
+      }
+    }
+    compiledWrapper.call(m.exports, m.exports, r, m)
+
+    const res = Object.prototype.hasOwnProperty.call(m.exports, 'default')
+      ? m.exports.default
+      : m.exports
+    evaluatedModules[filename] = res
+    return res
+  }
+  return evaluateModule
 }
 
-export default function runInVm (entry, files, _context = {}) {
-  return new Promise((resolve, reject) => {
+export default function runInVm (entry, files) {
+  const evaluate = compileModule(files)
+  return (_context = {}) => new Promise((resolve, reject) => {
     const context = createContext(_context)
-    const res = evaluateModule(entry, files, context, {})
+    const res = evaluate(entry, context, {})
     resolve(typeof res === 'function' ? res(_context) : res)
   })
 }