Skip to content

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

Rodrigo Muino Tomonari requested to merge github/fork/joyeecheung/fix-vm-measure into master

https://github.com/nodejs/node-v8/pull/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: https://github.com/nodejs/node-v8/pull/147

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Merge request reports