vm: fix vm.measureMemory() and introduce execution option

nodejs/node-v8#147 broke the
`vm.measureMemory()` API. It only created a `MeasureMemoryDelegate` and
without actually calling `v8::Isolate::MeasureMemory()` so the returned
promise will never resolve. This was not caught by the tests because
the promise resolvers were not wrapped with `common.mustCall()`.

This patch migrates the API properly and also introduce the newly
added execution option to the API. It also removes support for
specifying contexts to measure - instead we'll just return the
measurements for all contexts in the detailed mode, which is
what the `performance.measureMemory()` prototype in V8 currently does.
We can consider implementing our own `v8::MeasureMemoryDelegate`
to select the target context in `ShouldMeasure()` in the future,
but then we'll also need to implement `MeasurementComplete()`
to assemble the result. For now it's probably too early to do that.

Since this API is still experimental (and guarded with a warning),
such breakage should be acceptable.

Refs: nodejs/node-v8#147

PR-URL: #32988
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: joyeecheung

doc/api/vm.md

@@ -303,44 +303,70 @@ added: v13.10.0
 
 > Stability: 1 - Experimental
 
-Measure the memory known to V8 and used by the current execution context
-or a specified context.
+Measure the memory known to V8 and used by all contexts known to the
+current V8 isolate, or the main context.
 
 * `options` {Object} Optional.
-  * `mode` {string} Either `'summary'` or `'detailed'`.
+  * `mode` {string} Either `'summary'` or `'detailed'`. In summary mode,
+    only the memory measured for the main context will be returned. In
+    detailed mode, the measure measured for all contexts known to the
+    current V8 isolate will be returned.
     **Default:** `'summary'`
-  * `context` {Object} Optional. A [contextified][] object returned
-    by `vm.createContext()`. If not specified, measure the memory
-    usage of the current context where `vm.measureMemory()` is invoked.
+  * `execution` {string} Either `'default'` or `'eager'`. With default
+    execution, the promise will not resolve until after the next scheduled
+    garbage collection starts, which may take a while (or never if the program
+    exits before the next GC). With eager execution, the GC will be started
+    right away to measure the memory.
+    **Default:** `'default'`
 * Returns: {Promise} If the memory is successfully measured the promise will
   resolve with an object containing information about the memory usage.
 
 The format of the object that the returned Promise may resolve with is
 specific to the V8 engine and may change from one version of V8 to the next.
 
 The returned result is different from the statistics returned by
-`v8.getHeapSpaceStatistics()` in that `vm.measureMemory()` measures
-the memory reachable by V8 from a specific context, while
-`v8.getHeapSpaceStatistics()` measures the memory used by an instance
-of V8 engine, which can switch among multiple contexts that reference
-objects in the heap of one engine.
+`v8.getHeapSpaceStatistics()` in that `vm.measureMemory()` measure the
+memory reachable by each V8 specific contexts in the current instance of
+the V8 engine, while the result of `v8.getHeapSpaceStatistics()` measure
+the memory occupied by each heap space in the current V8 instance.
 
 ```js
 const vm = require('vm');
-// Measure the memory used by the current context and return the result
-// in summary.
+// Measure the memory used by the main context.
 vm.measureMemory({ mode: 'summary' })
-  // Is the same as vm.measureMemory()
+  // This is the same as vm.measureMemory()
   .then((result) => {
     // The current format is:
-    // { total: { jsMemoryEstimate: 2211728, jsMemoryRange: [ 0, 2211728 ] } }
+    // {
+    //   total: {
+    //      jsMemoryEstimate: 2418479, jsMemoryRange: [ 2418479, 2745799 ]
+    //    }
+    // }
     console.log(result);
   });
 
-const context = vm.createContext({});
-vm.measureMemory({ mode: 'detailed' }, context)
+const context = vm.createContext({ a: 1 });
+vm.measureMemory({ mode: 'detailed', execution: 'eager' })
   .then((result) => {
-    // At the moment the detailed format is the same as the summary one.
+    // Reference the context here so that it won't be GC'ed
+    // until the measurement is complete.
+    console.log(context.a);
+    // {
+    //   total: {
+    //     jsMemoryEstimate: 2574732,
+    //     jsMemoryRange: [ 2574732, 2904372 ]
+    //   },
+    //   current: {
+    //     jsMemoryEstimate: 2438996,
+    //     jsMemoryRange: [ 2438996, 2768636 ]
+    //   },
+    //   other: [
+    //     {
+    //       jsMemoryEstimate: 135736,
+    //       jsMemoryRange: [ 135736, 465376 ]
+    //     }
+    //   ]
+    // }
     console.log(result);
   });
 ```

lib/vm.js

@@ -385,20 +385,27 @@ const measureMemoryModes = {
   detailed: constants.measureMemory.mode.DETAILED,
 };
 
+const measureMemoryExecutions = {
+  default: constants.measureMemory.execution.DEFAULT,
+  eager: constants.measureMemory.execution.EAGER,
+};
+
 function measureMemory(options = {}) {
   emitExperimentalWarning('vm.measureMemory');
   validateObject(options, 'options');
-  const { mode = 'summary', context } = options;
+  const { mode = 'summary', execution = 'default' } = options;
   if (mode !== 'summary' && mode !== 'detailed') {
     throw new ERR_INVALID_ARG_VALUE(
       'options.mode', options.mode,
       'must be either \'summary\' or \'detailed\'');
   }
-  if (context !== undefined &&
-    (typeof context !== 'object' || context === null || !_isContext(context))) {
-    throw new ERR_INVALID_ARG_TYPE('options.context', 'vm.Context', context);
+  if (execution !== 'default' && execution !== 'eager') {
+    throw new ERR_INVALID_ARG_VALUE(
+      'options.execution', options.execution,
+      'must be either \'default\' or \'eager\'');
   }
-  const result = _measureMemory(measureMemoryModes[mode], context);
+  const result = _measureMemory(measureMemoryModes[mode],
+                                measureMemoryExecutions[execution]);
   if (result === undefined) {
     return PromiseReject(new ERR_CONTEXT_NOT_INITIALIZED());
   }

src/node_contextify.cc

@@ -53,6 +53,7 @@ using v8::Isolate;
 using v8::Local;
 using v8::Maybe;
 using v8::MaybeLocal;
+using v8::MeasureMemoryExecution;
 using v8::MeasureMemoryMode;
 using v8::Name;
 using v8::NamedPropertyHandlerConfiguration;
@@ -1211,29 +1212,22 @@ static void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
 
 static void MeasureMemory(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsInt32());
+  CHECK(args[1]->IsInt32());
   int32_t mode = args[0].As<v8::Int32>()->Value();
+  int32_t execution = args[1].As<v8::Int32>()->Value();
   Isolate* isolate = args.GetIsolate();
-  Environment* env = Environment::GetCurrent(args);
-  Local<Context> context;
-  if (args[1]->IsUndefined()) {
-    context = isolate->GetCurrentContext();
-  } else {
-    CHECK(args[1]->IsObject());
-    ContextifyContext* sandbox =
-        ContextifyContext::ContextFromContextifiedSandbox(env,
-                                                          args[1].As<Object>());
-    CHECK_NOT_NULL(sandbox);
-    context = sandbox->context();
-    if (context.IsEmpty()) {  // Not yet fully initialized
-      return;
-    }
-  }
+
+  Local<Context> current_context = isolate->GetCurrentContext();
   Local<Promise::Resolver> resolver;
-  if (!Promise::Resolver::New(context).ToLocal(&resolver)) return;
-  std::unique_ptr<v8::MeasureMemoryDelegate> i =
+  if (!Promise::Resolver::New(current_context).ToLocal(&resolver)) return;
+  std::unique_ptr<v8::MeasureMemoryDelegate> delegate =
       v8::MeasureMemoryDelegate::Default(
-          isolate, context, resolver, static_cast<v8::MeasureMemoryMode>(mode));
-  CHECK_NOT_NULL(i);
+          isolate,
+          current_context,
+          resolver,
+          static_cast<v8::MeasureMemoryMode>(mode));
+  isolate->MeasureMemory(std::move(delegate),
+                         static_cast<v8::MeasureMemoryExecution>(execution));
   v8::Local<v8::Promise> promise = resolver->GetPromise();
 
   args.GetReturnValue().Set(promise);
@@ -1265,13 +1259,27 @@ void Initialize(Local<Object> target,
 
   Local<Object> constants = Object::New(env->isolate());
   Local<Object> measure_memory = Object::New(env->isolate());
-  Local<Object> memory_mode = Object::New(env->isolate());
-  MeasureMemoryMode SUMMARY = MeasureMemoryMode::kSummary;
-  MeasureMemoryMode DETAILED = MeasureMemoryMode::kDetailed;
-  NODE_DEFINE_CONSTANT(memory_mode, SUMMARY);
-  NODE_DEFINE_CONSTANT(memory_mode, DETAILED);
-  READONLY_PROPERTY(measure_memory, "mode", memory_mode);
+  Local<Object> memory_execution = Object::New(env->isolate());
+
+  {
+    Local<Object> memory_mode = Object::New(env->isolate());
+    MeasureMemoryMode SUMMARY = MeasureMemoryMode::kSummary;
+    MeasureMemoryMode DETAILED = MeasureMemoryMode::kDetailed;
+    NODE_DEFINE_CONSTANT(memory_mode, SUMMARY);
+    NODE_DEFINE_CONSTANT(memory_mode, DETAILED);
+    READONLY_PROPERTY(measure_memory, "mode", memory_mode);
+  }
+
+  {
+    MeasureMemoryExecution DEFAULT = MeasureMemoryExecution::kDefault;
+    MeasureMemoryExecution EAGER = MeasureMemoryExecution::kEager;
+    NODE_DEFINE_CONSTANT(memory_execution, DEFAULT);
+    NODE_DEFINE_CONSTANT(memory_execution, EAGER);
+    READONLY_PROPERTY(measure_memory, "execution", memory_execution);
+  }
+
   READONLY_PROPERTY(constants, "measureMemory", measure_memory);
+
   target->Set(context, env->constants_string(), constants).Check();
 
   env->SetMethod(target, "measureMemory", MeasureMemory);

test/common/measure-memory.js

@@ -0,0 +1,57 @@
+/* eslint-disable node-core/require-common-first, node-core/required-modules */
+'use strict';
+
+const assert = require('assert');
+const common = require('./');
+
+// The formats could change when V8 is updated, then the tests should be
+// updated accordingly.
+function assertResultShape(result) {
+  assert.strictEqual(typeof result.jsMemoryEstimate, 'number');
+  assert.strictEqual(typeof result.jsMemoryRange[0], 'number');
+  assert.strictEqual(typeof result.jsMemoryRange[1], 'number');
+}
+
+function assertSummaryShape(result) {
+  assert.strictEqual(typeof result, 'object');
+  assert.strictEqual(typeof result.total, 'object');
+  assertResultShape(result.total);
+}
+
+function assertDetailedShape(result, contexts = 0) {
+  assert.strictEqual(typeof result, 'object');
+  assert.strictEqual(typeof result.total, 'object');
+  assert.strictEqual(typeof result.current, 'object');
+  assertResultShape(result.total);
+  assertResultShape(result.current);
+  if (contexts === 0) {
+    assert.deepStrictEqual(result.other, []);
+  } else {
+    assert.strictEqual(result.other.length, contexts);
+    for (const item of result.other) {
+      assertResultShape(item);
+    }
+  }
+}
+
+function assertSingleDetailedShape(result) {
+  assert.strictEqual(typeof result, 'object');
+  assert.strictEqual(typeof result.total, 'object');
+  assert.strictEqual(typeof result.current, 'object');
+  assert.deepStrictEqual(result.other, []);
+  assertResultShape(result.total);
+  assertResultShape(result.current);
+}
+
+function expectExperimentalWarning() {
+  common.expectWarning('ExperimentalWarning',
+                       'vm.measureMemory is an experimental feature. ' +
+                       'This feature could change at any time');
+}
+
+module.exports = {
+  assertSummaryShape,
+  assertDetailedShape,
+  assertSingleDetailedShape,
+  expectExperimentalWarning
+};

test/parallel/test-vm-measure-memory-lazy.js

@@ -0,0 +1,37 @@
+// Flags: --expose-gc
+
+'use strict';
+const common = require('../common');
+const {
+  assertSummaryShape,
+  expectExperimentalWarning
+} = require('../common/measure-memory');
+const vm = require('vm');
+
+expectExperimentalWarning();
+
+// Test lazy memory measurement - we will need to global.gc()
+// or otherwise these may not resolve.
+{
+  vm.measureMemory()
+    .then(common.mustCall(assertSummaryShape));
+  global.gc();
+}
+
+{
+  vm.measureMemory({})
+    .then(common.mustCall(assertSummaryShape));
+  global.gc();
+}
+
+{
+  vm.measureMemory({ mode: 'summary' })
+    .then(common.mustCall(assertSummaryShape));
+  global.gc();
+}
+
+{
+  vm.measureMemory({ mode: 'detailed' })
+    .then(common.mustCall(assertSummaryShape));
+  global.gc();
+}

test/parallel/test-vm-measure-memory-multi-context.js

@@ -0,0 +1,28 @@
+'use strict';
+const common = require('../common');
+const {
+  assertDetailedShape,
+  expectExperimentalWarning
+} = require('../common/measure-memory');
+const vm = require('vm');
+const assert = require('assert');
+
+expectExperimentalWarning();
+{
+  const arr = [];
+  const count = 10;
+  for (let i = 0; i < count; ++i) {
+    const context = vm.createContext({
+      test: new Array(100).fill('foo')
+    });
+    arr.push(context);
+  }
+  // Check that one more context shows up in the result
+  vm.measureMemory({ mode: 'detailed', execution: 'eager' })
+    .then(common.mustCall((result) => {
+      // We must hold on to the contexts here so that they
+      // don't get GC'ed until the measurement is complete
+      assert.strictEqual(arr.length, count);
+      assertDetailedShape(result, count);
+    }));
+}