Unverified Commit 5f2c4ce7 authored by Joyee Cheung's avatar Joyee Cheung
Browse files

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

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

PR-URL: https://github.com/nodejs/node/pull/32988


Reviewed-By: default avatarBen Noordhuis <info@bnoordhuis.nl>
Reviewed-By: default avatarJiawen Geng <technicalcute@gmail.com>
Reviewed-By: default avatarJames M Snell <jasnell@gmail.com>
parent 0bd24a6e
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment