Skip to content

Commit 5f2c4ce

Browse files
joyeecheungjoyeecheung
committed
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: nodejs#32988 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0bd24a6 commit 5f2c4ce

File tree

7 files changed

+226
-97
lines changed

7 files changed

+226
-97
lines changed

‎doc/api/vm.md‎

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -303,44 +303,70 @@ added: v13.10.0
303303

304304
> Stability: 1 - Experimental
305305
306-
Measure the memory known to V8 and used by the current execution context
307-
or a specified context.
306+
Measure the memory known to V8 and used by all contexts known to the
307+
current V8 isolate, or the main context.
308308

309309
* `options` {Object} Optional.
310-
* `mode` {string} Either `'summary'` or `'detailed'`.
310+
* `mode` {string} Either `'summary'` or `'detailed'`. In summary mode,
311+
only the memory measured for the main context will be returned. In
312+
detailed mode, the measure measured for all contexts known to the
313+
current V8 isolate will be returned.
311314
**Default:** `'summary'`
312-
* `context` {Object} Optional. A [contextified][] object returned
313-
by `vm.createContext()`. If not specified, measure the memory
314-
usage of the current context where `vm.measureMemory()` is invoked.
315+
* `execution` {string} Either `'default'` or `'eager'`. With default
316+
execution, the promise will not resolve until after the next scheduled
317+
garbage collection starts, which may take a while (or never if the program
318+
exits before the next GC). With eager execution, the GC will be started
319+
right away to measure the memory.
320+
**Default:** `'default'`
315321
* Returns: {Promise} If the memory is successfully measured the promise will
316322
resolve with an object containing information about the memory usage.
317323

318324
The format of the object that the returned Promise may resolve with is
319325
specific to the V8 engine and may change from one version of V8 to the next.
320326

321327
The returned result is different from the statistics returned by
322-
`v8.getHeapSpaceStatistics()` in that `vm.measureMemory()` measures
323-
the memory reachable by V8 from a specific context, while
324-
`v8.getHeapSpaceStatistics()` measures the memory used by an instance
325-
of V8 engine, which can switch among multiple contexts that reference
326-
objects in the heap of one engine.
328+
`v8.getHeapSpaceStatistics()` in that `vm.measureMemory()` measure the
329+
memory reachable by each V8 specific contexts in the current instance of
330+
the V8 engine, while the result of `v8.getHeapSpaceStatistics()` measure
331+
the memory occupied by each heap space in the current V8 instance.
327332

328333
```js
329334
const vm = require('vm');
330-
// Measure the memory used by the current context and return the result
331-
// in summary.
335+
// Measure the memory used by the main context.
332336
vm.measureMemory({ mode: 'summary' })
333-
// Is the same as vm.measureMemory()
337+
// This is the same as vm.measureMemory()
334338
.then((result) => {
335339
// The current format is:
336-
// { total: { jsMemoryEstimate: 2211728, jsMemoryRange: [ 0, 2211728 ] } }
340+
// {
341+
// total: {
342+
// jsMemoryEstimate: 2418479, jsMemoryRange: [ 2418479, 2745799 ]
343+
// }
344+
// }
337345
console.log(result);
338346
});
339347

340-
const context = vm.createContext({});
341-
vm.measureMemory({ mode: 'detailed' }, context)
348+
const context = vm.createContext({ a: 1 });
349+
vm.measureMemory({ mode: 'detailed', execution: 'eager' })
342350
.then((result) => {
343-
// At the moment the detailed format is the same as the summary one.
351+
// Reference the context here so that it won't be GC'ed
352+
// until the measurement is complete.
353+
console.log(context.a);
354+
// {
355+
// total: {
356+
// jsMemoryEstimate: 2574732,
357+
// jsMemoryRange: [ 2574732, 2904372 ]
358+
// },
359+
// current: {
360+
// jsMemoryEstimate: 2438996,
361+
// jsMemoryRange: [ 2438996, 2768636 ]
362+
// },
363+
// other: [
364+
// {
365+
// jsMemoryEstimate: 135736,
366+
// jsMemoryRange: [ 135736, 465376 ]
367+
// }
368+
// ]
369+
// }
344370
console.log(result);
345371
});
346372
```

‎lib/vm.js‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -385,20 +385,27 @@ const measureMemoryModes = {
385385
detailed: constants.measureMemory.mode.DETAILED,
386386
};
387387

388+
const measureMemoryExecutions = {
389+
default: constants.measureMemory.execution.DEFAULT,
390+
eager: constants.measureMemory.execution.EAGER,
391+
};
392+
388393
function measureMemory(options = {}) {
389394
emitExperimentalWarning('vm.measureMemory');
390395
validateObject(options, 'options');
391-
const { mode = 'summary', context } = options;
396+
const { mode = 'summary', execution = 'default' } = options;
392397
if (mode !== 'summary' && mode !== 'detailed') {
393398
throw new ERR_INVALID_ARG_VALUE(
394399
'options.mode', options.mode,
395400
'must be either \'summary\' or \'detailed\'');
396401
}
397-
if (context !== undefined &&
398-
(typeof context !== 'object' || context === null || !_isContext(context))) {
399-
throw new ERR_INVALID_ARG_TYPE('options.context', 'vm.Context', context);
402+
if (execution !== 'default' && execution !== 'eager') {
403+
throw new ERR_INVALID_ARG_VALUE(
404+
'options.execution', options.execution,
405+
'must be either \'default\' or \'eager\'');
400406
}
401-
const result = _measureMemory(measureMemoryModes[mode], context);
407+
const result = _measureMemory(measureMemoryModes[mode],
408+
measureMemoryExecutions[execution]);
402409
if (result === undefined) {
403410
return PromiseReject(new ERR_CONTEXT_NOT_INITIALIZED());
404411
}

‎src/node_contextify.cc‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ using v8::Isolate;
5353
using v8::Local;
5454
using v8::Maybe;
5555
using v8::MaybeLocal;
56+
using v8::MeasureMemoryExecution;
5657
using v8::MeasureMemoryMode;
5758
using v8::Name;
5859
using v8::NamedPropertyHandlerConfiguration;
@@ -1211,29 +1212,22 @@ static void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
12111212

12121213
static void MeasureMemory(const FunctionCallbackInfo<Value>& args) {
12131214
CHECK(args[0]->IsInt32());
1215+
CHECK(args[1]->IsInt32());
12141216
int32_t mode = args[0].As<v8::Int32>()->Value();
1217+
int32_t execution = args[1].As<v8::Int32>()->Value();
12151218
Isolate* isolate = args.GetIsolate();
1216-
Environment* env = Environment::GetCurrent(args);
1217-
Local<Context> context;
1218-
if (args[1]->IsUndefined()) {
1219-
context = isolate->GetCurrentContext();
1220-
} else {
1221-
CHECK(args[1]->IsObject());
1222-
ContextifyContext* sandbox =
1223-
ContextifyContext::ContextFromContextifiedSandbox(env,
1224-
args[1].As<Object>());
1225-
CHECK_NOT_NULL(sandbox);
1226-
context = sandbox->context();
1227-
if (context.IsEmpty()) { // Not yet fully initialized
1228-
return;
1229-
}
1230-
}
1219+
1220+
Local<Context> current_context = isolate->GetCurrentContext();
12311221
Local<Promise::Resolver> resolver;
1232-
if (!Promise::Resolver::New(context).ToLocal(&resolver)) return;
1233-
std::unique_ptr<v8::MeasureMemoryDelegate> i =
1222+
if (!Promise::Resolver::New(current_context).ToLocal(&resolver)) return;
1223+
std::unique_ptr<v8::MeasureMemoryDelegate> delegate =
12341224
v8::MeasureMemoryDelegate::Default(
1235-
isolate, context, resolver, static_cast<v8::MeasureMemoryMode>(mode));
1236-
CHECK_NOT_NULL(i);
1225+
isolate,
1226+
current_context,
1227+
resolver,
1228+
static_cast<v8::MeasureMemoryMode>(mode));
1229+
isolate->MeasureMemory(std::move(delegate),
1230+
static_cast<v8::MeasureMemoryExecution>(execution));
12371231
v8::Local<v8::Promise> promise = resolver->GetPromise();
12381232

12391233
args.GetReturnValue().Set(promise);
@@ -1265,13 +1259,27 @@ void Initialize(Local<Object> target,
12651259

12661260
Local<Object> constants = Object::New(env->isolate());
12671261
Local<Object> measure_memory = Object::New(env->isolate());
1268-
Local<Object> memory_mode = Object::New(env->isolate());
1269-
MeasureMemoryMode SUMMARY = MeasureMemoryMode::kSummary;
1270-
MeasureMemoryMode DETAILED = MeasureMemoryMode::kDetailed;
1271-
NODE_DEFINE_CONSTANT(memory_mode, SUMMARY);
1272-
NODE_DEFINE_CONSTANT(memory_mode, DETAILED);
1273-
READONLY_PROPERTY(measure_memory, "mode", memory_mode);
1262+
Local<Object> memory_execution = Object::New(env->isolate());
1263+
1264+
{
1265+
Local<Object> memory_mode = Object::New(env->isolate());
1266+
MeasureMemoryMode SUMMARY = MeasureMemoryMode::kSummary;
1267+
MeasureMemoryMode DETAILED = MeasureMemoryMode::kDetailed;
1268+
NODE_DEFINE_CONSTANT(memory_mode, SUMMARY);
1269+
NODE_DEFINE_CONSTANT(memory_mode, DETAILED);
1270+
READONLY_PROPERTY(measure_memory, "mode", memory_mode);
1271+
}
1272+
1273+
{
1274+
MeasureMemoryExecution DEFAULT = MeasureMemoryExecution::kDefault;
1275+
MeasureMemoryExecution EAGER = MeasureMemoryExecution::kEager;
1276+
NODE_DEFINE_CONSTANT(memory_execution, DEFAULT);
1277+
NODE_DEFINE_CONSTANT(memory_execution, EAGER);
1278+
READONLY_PROPERTY(measure_memory, "execution", memory_execution);
1279+
}
1280+
12741281
READONLY_PROPERTY(constants, "measureMemory", measure_memory);
1282+
12751283
target->Set(context, env->constants_string(), constants).Check();
12761284

12771285
env->SetMethod(target, "measureMemory", MeasureMemory);

‎test/common/measure-memory.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/* eslint-disable node-core/require-common-first, node-core/required-modules */
2+
'use strict';
3+
4+
const assert = require('assert');
5+
const common = require('./');
6+
7+
// The formats could change when V8 is updated, then the tests should be
8+
// updated accordingly.
9+
function assertResultShape(result) {
10+
assert.strictEqual(typeof result.jsMemoryEstimate, 'number');
11+
assert.strictEqual(typeof result.jsMemoryRange[0], 'number');
12+
assert.strictEqual(typeof result.jsMemoryRange[1], 'number');
13+
}
14+
15+
function assertSummaryShape(result) {
16+
assert.strictEqual(typeof result, 'object');
17+
assert.strictEqual(typeof result.total, 'object');
18+
assertResultShape(result.total);
19+
}
20+
21+
function assertDetailedShape(result, contexts = 0) {
22+
assert.strictEqual(typeof result, 'object');
23+
assert.strictEqual(typeof result.total, 'object');
24+
assert.strictEqual(typeof result.current, 'object');
25+
assertResultShape(result.total);
26+
assertResultShape(result.current);
27+
if (contexts === 0) {
28+
assert.deepStrictEqual(result.other, []);
29+
} else {
30+
assert.strictEqual(result.other.length, contexts);
31+
for (const item of result.other) {
32+
assertResultShape(item);
33+
}
34+
}
35+
}
36+
37+
function assertSingleDetailedShape(result) {
38+
assert.strictEqual(typeof result, 'object');
39+
assert.strictEqual(typeof result.total, 'object');
40+
assert.strictEqual(typeof result.current, 'object');
41+
assert.deepStrictEqual(result.other, []);
42+
assertResultShape(result.total);
43+
assertResultShape(result.current);
44+
}
45+
46+
function expectExperimentalWarning() {
47+
common.expectWarning('ExperimentalWarning',
48+
'vm.measureMemory is an experimental feature. ' +
49+
'This feature could change at any time');
50+
}
51+
52+
module.exports = {
53+
assertSummaryShape,
54+
assertDetailedShape,
55+
assertSingleDetailedShape,
56+
expectExperimentalWarning
57+
};

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Flags: --expose-gc
2+
3+
'use strict';
4+
const common = require('../common');
5+
const {
6+
assertSummaryShape,
7+
expectExperimentalWarning
8+
} = require('../common/measure-memory');
9+
const vm = require('vm');
10+
11+
expectExperimentalWarning();
12+
13+
// Test lazy memory measurement - we will need to global.gc()
14+
// or otherwise these may not resolve.
15+
{
16+
vm.measureMemory()
17+
.then(common.mustCall(assertSummaryShape));
18+
global.gc();
19+
}
20+
21+
{
22+
vm.measureMemory({})
23+
.then(common.mustCall(assertSummaryShape));
24+
global.gc();
25+
}
26+
27+
{
28+
vm.measureMemory({ mode: 'summary' })
29+
.then(common.mustCall(assertSummaryShape));
30+
global.gc();
31+
}
32+
33+
{
34+
vm.measureMemory({ mode: 'detailed' })
35+
.then(common.mustCall(assertSummaryShape));
36+
global.gc();
37+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
const common = require('../common');
3+
const {
4+
assertDetailedShape,
5+
expectExperimentalWarning
6+
} = require('../common/measure-memory');
7+
const vm = require('vm');
8+
const assert = require('assert');
9+
10+
expectExperimentalWarning();
11+
{
12+
const arr = [];
13+
const count = 10;
14+
for (let i = 0; i < count; ++i) {
15+
const context = vm.createContext({
16+
test: new Array(100).fill('foo')
17+
});
18+
arr.push(context);
19+
}
20+
// Check that one more context shows up in the result
21+
vm.measureMemory({ mode: 'detailed', execution: 'eager' })
22+
.then(common.mustCall((result) => {
23+
// We must hold on to the contexts here so that they
24+
// don't get GC'ed until the measurement is complete
25+
assert.strictEqual(arr.length, count);
26+
assertDetailedShape(result, count);
27+
}));
28+
}

0 commit comments

Comments
 (0)